From owner-freebsd-hackers@FreeBSD.ORG Thu Jan 12 12:52:01 2012 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 67851106566B; Thu, 12 Jan 2012 12:52:01 +0000 (UTC) (envelope-from lists@eitanadler.com) Received: from mail-lpp01m010-f54.google.com (mail-lpp01m010-f54.google.com [209.85.215.54]) by mx1.freebsd.org (Postfix) with ESMTP id 6F1808FC17; Thu, 12 Jan 2012 12:52:00 +0000 (UTC) Received: by lahd3 with SMTP id d3so1256998lah.13 for ; Thu, 12 Jan 2012 04:51:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=eitanadler.com; s=0xdeadbeef; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:content-transfer-encoding; bh=Vg5tXhtcSWKSqyukNofkg6XEgJfAL18lfJoZqiI4YEc=; b=OU2nkrNKGWbOhKamlUwUCya2CefzCkJ6QCNdWvDcUsxA0p/CwnkJ91u3LoaezdLNOF gfPy0Urmsto8JVdU9pRuzNZbtzRKCjeMg5oVj98NsZVDNbn4wep+8sHlaEaUXQtvKFeF E3H0mKbpG2JYnQ/DkrnScvxsiy9yUpGi2OJtc= Received: by 10.112.25.35 with SMTP id z3mr772582lbf.52.1326372719170; Thu, 12 Jan 2012 04:51:59 -0800 (PST) MIME-Version: 1.0 Received: by 10.152.129.8 with HTTP; Thu, 12 Jan 2012 04:51:28 -0800 (PST) In-Reply-To: <20120112100840.GV31224@deviant.kiev.zoral.com.ua> References: <20120112100840.GV31224@deviant.kiev.zoral.com.ua> From: Eitan Adler Date: Thu, 12 Jan 2012 07:51:28 -0500 Message-ID: To: Kostik Belousov Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: jilles@freebsd.org, FreeBSD Hackers , Colin Percival Subject: Re: dup3 syscall - atomic set O_CLOEXEC with dup2 X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Jan 2012 12:52:01 -0000 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 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 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