This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Patches to improve Hurd parts: dup3


Hello!

On Wed, Jan 07, 2009 at 07:01:13PM -0800, Roland McGrath wrote:
> Please resend a patch implementing dup3 against the current sources.

The sources haven't changed, but I think you mean to have me aggregate my
later follow-up patch together with the original patch into a fresh one?


First, we need this generic patch:

2009-01-08  Thomas Schwinge  <tschwinge@gnu.org>

        * include/unistd.h (__dup3): New hidden declaration.
	* io/dup3.c (dup3): Rename to __dup3 and add a weak alias.
	* sysdeps/unix/syscalls.list: Likewise.

Index: include/unistd.h
===================================================================
RCS file: /cvs/glibc/libc/include/unistd.h,v
retrieving revision 1.50
diff -u -p -r1.50 unistd.h
--- include/unistd.h	27 Jul 2008 18:23:17 -0000	1.50
+++ include/unistd.h	8 Jan 2009 13:48:52 -0000
@@ -75,6 +75,8 @@ char *__canonicalize_directory_name_inte
 extern int __dup (int __fd);
 extern int __dup2 (int __fd, int __fd2);
 libc_hidden_proto (__dup2)
+extern int __dup3 (int __fd, int __fd2, int __flags);
+libc_hidden_proto (__dup3)
 extern int __execve (__const char *__path, char *__const __argv[],
 		     char *__const __envp[]);
 extern long int __pathconf (__const char *__path, int __name);
Index: io/dup3.c
===================================================================
RCS file: /cvs/glibc/libc/io/dup3.c,v
retrieving revision 1.1
diff -u -p -r1.1 dup3.c
--- io/dup3.c	25 Jul 2008 04:32:48 -0000	1.1
+++ io/dup3.c	8 Jan 2009 13:48:52 -0000
@@ -25,7 +25,7 @@
    open the same file as FD is which setting flags according to
    FLAGS.  Return FD2 or -1.  */
 int
-dup3 (fd, fd2, flags)
+__dup3 (fd, fd2, flags)
      int fd;
      int fd2;
      int flags;
@@ -43,6 +43,8 @@ dup3 (fd, fd2, flags)
   __set_errno (ENOSYS);
   return -1;
 }
+libc_hidden_def (__dup3)
 stub_warning (dup3)
 
+weak_alias (__dup3, dup3)
 #include <stub-tag.h>
Index: sysdeps/unix/syscalls.list
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/unix/syscalls.list,v
retrieving revision 1.33
diff -u -p -r1.33 syscalls.list
--- sysdeps/unix/syscalls.list	25 Jul 2008 04:38:07 -0000	1.33
+++ sysdeps/unix/syscalls.list	8 Jan 2009 13:48:58 -0000
@@ -9,7 +9,7 @@ chroot		-	chroot		i:s	chroot
 close		-	close		Ci:i	__libc_close	__close close
 dup		-	dup		i:i	__dup		dup
 dup2		-	dup2		i:ii	__dup2		dup2
-dup3		-	dup3		i:iii	dup3
+dup3		-	dup3		i:iii	__dup3		dup3
 fchdir		-	fchdir		i:i	__fchdir	fchdir
 fcntl		-	fcntl		Ci:iiF	__libc_fcntl	__fcntl __fcntl_internal fcntl
 fstatfs		-	fstatfs		i:ip	__fstatfs	fstatfs


And then the Hurd changes on top of that:

2009-01-08  Thomas Schwinge  <tschwinge@gnu.org>

	* sysdeps/mach/hurd/dup3.c: New file, copy from dup2.c.  Evolve it to
	implement __dup3 and do some further code clean-ups.
	* sysdeps/mach/hurd/dup2.c (__dup2): Reimplement using __dup3.

Recipe:

Copy sysdeps/mach/hurd/dup2.c to sysdeps/mach/hurd/dup3.c.

Evolve the latter one to implement dup3:

--- sysdeps/mach/hurd/dup2.c.O	2008-12-17 00:55:30.000000000 +0100
+++ sysdeps/mach/hurd/dup3.c	2008-12-17 15:27:40.000000000 +0100
@@ -23,21 +23,28 @@
 #include <hurd/fd.h>
 
 /* Duplicate FD to FD2, closing the old FD2 and making FD2 be
-   open on the same file as FD is.  Return FD2 or -1.  */
+   open the same file as FD is which setting flags according to
+   FLAGS.  Return FD2 or -1.  */
 int
-__dup2 (fd, fd2)
+__dup3 (fd, fd2, flags)
      int fd;
      int fd2;
+     int flags;
 {
   struct hurd_fd *d;
 
+  /* Both passing flags different from O_CLOEXEC and FD2 being the same as FD
+     are invalid.  */
+  if ((flags & ~O_CLOEXEC || fd2 == fd) &&
+      /* ... with the exception in case that dup2 behavior is requested: if FD
+	 is valid and FD2 is already the same then just return it.  */	 
+      ! (flags == -1 && fd2 == fd))
+    return __hurd_fail (EINVAL);
+
   /* Extract the ports and flags from FD.  */
   d = _hurd_fd_get (fd);
   if (d == NULL)
-    {
-      errno = EBADF;
-      return -1;
-    }
+    return __hurd_fail (EBADF);
 
   HURD_CRITICAL_BEGIN;
 
@@ -45,24 +52,19 @@ __dup2 (fd, fd2)
   if (d->port.port == MACH_PORT_NULL)
     {
       __spin_unlock (&d->port.lock);
-      errno = EBADF;
-      fd2 = -1;
+      fd2 = __hurd_fail (EBADF);
     }
   else if (fd2 == fd)
-    /* FD is valid and FD2 is already the same; just return it.  */
     __spin_unlock (&d->port.lock);
   else
     {
       struct hurd_userlink ulink, ctty_ulink;
-      int flags = d->flags;
+      int d_flags = d->flags;
       io_t ctty = _hurd_port_get (&d->ctty, &ctty_ulink);
       io_t port = _hurd_port_locked_get (&d->port, &ulink); /* Unlocks D.  */
 
       if (fd2 < 0)
-	{
-	  errno = EBADF;
-	  fd2 = -1;
-	}
+	fd2 = __hurd_fail (EBADF);
       else
 	{
 	  /* Get a hold of the destination descriptor.  */
@@ -114,7 +116,11 @@ __dup2 (fd, fd2)
 
 	      /* Install the ports and flags in the new descriptor slot.  */
 	      __spin_lock (&d2->port.lock);
-	      d2->flags = flags & ~FD_CLOEXEC; /* Dup clears FD_CLOEXEC. */
+	      if (flags & O_CLOEXEC)
+		d2->flags = d_flags | FD_CLOEXEC;
+	      else
+		/* dup clears FD_CLOEXEC.  */
+		d2->flags = d_flags & ~FD_CLOEXEC;
 	      _hurd_port_set (&d2->ctty, ctty);
 	      _hurd_port_locked_set (&d2->port, port); /* Unlocks D2.  */
 	    }
@@ -130,5 +136,5 @@ __dup2 (fd, fd2)
 
   return fd2;
 }
-libc_hidden_def (__dup2)
-weak_alias (__dup2, dup2)
+libc_hidden_def (__dup3)
+weak_alias (__dup3, dup3)

Reimplement __dup2 using __dup3:

diff --git a/sysdeps/mach/hurd/dup2.c b/sysdeps/mach/hurd/dup2.c
index 3abd30c..1ec8cb6 100644
--- sysdeps/mach/hurd/dup2.c
+++ sysdeps/mach/hurd/dup2.c
@@ -16,11 +16,7 @@
    Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
    02111-1307 USA.  */
 
-#include <errno.h>
-#include <fcntl.h>
 #include <unistd.h>
-#include <hurd.h>
-#include <hurd/fd.h>
 
 /* Duplicate FD to FD2, closing the old FD2 and making FD2 be
    open on the same file as FD is.  Return FD2 or -1.  */
@@ -29,106 +25,13 @@ __dup2 (fd, fd2)
      int fd;
      int fd2;
 {
-  struct hurd_fd *d;
+  int flags = 0;
 
-  /* Extract the ports and flags from FD.  */
-  d = _hurd_fd_get (fd);
-  if (d == NULL)
-    {
-      errno = EBADF;
-      return -1;
-    }
+  if (fd2 == fd)
+    /* See the comment in dup3.  */
+    flags = -1;
 
-  HURD_CRITICAL_BEGIN;
-
-  __spin_lock (&d->port.lock);
-  if (d->port.port == MACH_PORT_NULL)
-    {
-      __spin_unlock (&d->port.lock);
-      errno = EBADF;
-      fd2 = -1;
-    }
-  else if (fd2 == fd)
-    /* FD is valid and FD2 is already the same; just return it.  */
-    __spin_unlock (&d->port.lock);
-  else
-    {
-      struct hurd_userlink ulink, ctty_ulink;
-      int flags = d->flags;
-      io_t ctty = _hurd_port_get (&d->ctty, &ctty_ulink);
-      io_t port = _hurd_port_locked_get (&d->port, &ulink); /* Unlocks D.  */
-
-      if (fd2 < 0)
-	{
-	  errno = EBADF;
-	  fd2 = -1;
-	}
-      else
-	{
-	  /* Get a hold of the destination descriptor.  */
-	  struct hurd_fd *d2;
-
-	  if (fd2 >= _hurd_dtablesize)
-	    {
-	      /* The table is not large enough to hold the destination
-		 descriptor.  Enlarge it as necessary to allocate this
-		 descriptor.  */
-	      __mutex_unlock (&_hurd_dtable_lock);
-	      /* We still hold FD1's lock, but this is safe because
-		 _hurd_alloc_fd will only examine the cells starting
-		 at FD2.  */
-	      d2 = _hurd_alloc_fd (NULL, fd2);
-	      if (d2)
-		__spin_unlock (&d2->port.lock);
-	      __mutex_lock (&_hurd_dtable_lock);
-	    }
-	  else
-	    {
-	      d2 = _hurd_dtable[fd2];
-	      if (d2 == NULL)
-		{
-		  /* Must allocate a new one.  We don't initialize the port
-		     cells with this call so that if it fails (out of
-		     memory), we will not have already added user
-		     references for the ports, which we would then have to
-		     deallocate.  */
-		  d2 = _hurd_dtable[fd2] = _hurd_new_fd (MACH_PORT_NULL,
-							 MACH_PORT_NULL);
-		}
-	    }
-
-	  if (d2 == NULL)
-	    {
-	      fd2 = -1;
-	      if (errno == EINVAL)
-		errno = EBADF;	/* POSIX.1-1990 6.2.1.2 ll 54-55.  */
-	    }
-	  else
-	    {
-	      /* Give the ports each a user ref for the new descriptor.  */
-	      __mach_port_mod_refs (__mach_task_self (), port,
-				    MACH_PORT_RIGHT_SEND, 1);
-	      if (ctty != MACH_PORT_NULL)
-		__mach_port_mod_refs (__mach_task_self (), ctty,
-				      MACH_PORT_RIGHT_SEND, 1);
-
-	      /* Install the ports and flags in the new descriptor slot.  */
-	      __spin_lock (&d2->port.lock);
-	      d2->flags = flags & ~FD_CLOEXEC; /* Dup clears FD_CLOEXEC. */
-	      _hurd_port_set (&d2->ctty, ctty);
-	      _hurd_port_locked_set (&d2->port, port); /* Unlocks D2.  */
-	    }
-	}
-      __mutex_unlock (&_hurd_dtable_lock);
-
-      _hurd_port_free (&d->port, &ulink, port);
-      if (ctty != MACH_PORT_NULL)
-	_hurd_port_free (&d->ctty, &ctty_ulink, port);
-    }
-
-  HURD_CRITICAL_END;
-
-  return fd2;
+  return __dup3 (fd, fd2, flags);
 }
 libc_hidden_def (__dup2)
 weak_alias (__dup2, dup2)


Regards,
 Thomas

Attachment: signature.asc
Description: Digital signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]