Date: Thu, 12 Jan 2012 07:51:28 -0500 From: Eitan Adler <lists@eitanadler.com> To: Kostik Belousov <kostikbel@gmail.com> Cc: jilles@freebsd.org, FreeBSD Hackers <freebsd-hackers@freebsd.org>, Colin Percival <cperciva@freebsd.org> Subject: Re: dup3 syscall - atomic set O_CLOEXEC with dup2 Message-ID: <CAF6rxgne7M9xBb-mM1xsjPy3r-O%2BO%2BMzuYrsweG849V83MX3mA@mail.gmail.com> In-Reply-To: <20120112100840.GV31224@deviant.kiev.zoral.com.ua> References: <CAF6rxg=EjkwFbXQt3i2Nnz6_dcZtdek-2YdqyZnAdVPxVaWR_Q@mail.gmail.com> <20120112100840.GV31224@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
Thank you for your comments. I will hopefully have version 2 of the diff tonight. On Thu, Jan 12, 2012 at 4:56 AM, Ed Schouten <ed@80386.nl> wrote: > I suspect that not long after we add dup3(), some random person asks us > to implement F_DUP3FD. Any chance you can implement this without using a > system call, but through fcntl()? The goal is to be atomic with the duplication. On Thu, Jan 12, 2012 at 5:08 AM, Kostik Belousov <kostikbel@gmail.com> wrot= e: > makesyscalls.sh. This makes me seriosly wonder was the patch compiled at > all. It was compiled and tested in a virtual machine. I did however generate this diff differently than the one I tested (I applied my git diff against svn and used the svn diff here) so I may have missed something when doing that. >> + >> + =C2=A0 =C2=A0 return (do_dup(td, dupflags, (int)uap->from, (int)uap->t= o, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 td->td_retval)= ); > Why casting arguments to int ? I stole this line from sys_dup2. >> + =C2=A0 =C2=A0 return (0); > Isn't this line never executed ? Yes, this was leftover of some previous version. >> +} >> + >> +/* >> =C2=A0 * Duplicate a file descriptor to a particular value. >> =C2=A0 * >> =C2=A0 * Note: keep in mind that a potential race condition exists when = closing >> @@ -912,6 +945,9 @@ >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fdp->fd_lastfile =3D ne= w; >> =C2=A0 =C2=A0 =C2=A0 *retval =3D new; >> >> + =C2=A0 =C2=A0 if (flags & DUP_CLOEXEC) >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fdp->fd_ofileflags[new] |=3D= UF_EXCLOSE; >> + > It is better to handle UF_EXCLOSE at the time of assignment to > fdp->fd_ofileflags[new] just above, instead of clearing UF_EXCLOSE > and then setting it. > > That said, I am not sure that we shall copy the O_CLOEXEC crusade from > Linux until it is standartized. And, if copying, I think we shall copy > all bits of the new API, like SOCK_CLOEXEC etc, and not just dup3. If it is standardized I can't see how it will have semantics that differ from the current Linux version and IMHO this is a worthwhile thing to add. > Oh, and it misses the versioning for the new syscall symbols in libc. > The symbols not listed in the Symbol.maps are made local during the > final linkage. Cut me a bit slack, this is my first attempt at adding a syscall ;) --=20 Eitan Adler
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAF6rxgne7M9xBb-mM1xsjPy3r-O%2BO%2BMzuYrsweG849V83MX3mA>