Re: dir_lookup, locking and EINTR

[Samuel Thibault]

Thomas Bushnell BSG, le Tue 19 Aug 2008 23:09:56 -0700, a écrit :
> Your patch (which I see has been applied) seems to be clearly the Wrong
> Thing in the case where NP == DNP.

Aow right.

> The right thing to do is to throw NEWPI away at the very end of the
> function.

That's safer indeed.  To make things simpler, the patch below just
systematically resets NEWPI and NEWPO to NULL as soon as they don't need
to be dereferenced, does it look ok?

Samuel

2008-08-21  Samuel Thibault  <samuel.thibault@ens-lyon.org>

        dir-lookup.c (diskfs_S_dir_lookup): Initialize NEWPI and NEWPO to NULL,
        set back to NULL when they do not need to be referenced any more,
        dereference them if needed just before exit, after all unlocking.

Index: libdiskfs/dir-lookup.c
===================================================================
RCS file: /cvsroot/hurd/hurd/libdiskfs/dir-lookup.c,v
retrieving revision 1.56
diff -u -p -r1.56 dir-lookup.c
--- libdiskfs/dir-lookup.c      20 Aug 2008 22:59:52 -0000      1.56
+++ libdiskfs/dir-lookup.c      20 Aug 2008 23:29:52 -0000
@@ -53,8 +53,8 @@ diskfs_S_dir_lookup (struct protid *dirc
   int mustbedir = 0;
   size_t amt;
   int type;
-  struct protid *newpi;
-  struct peropen *newpo;
+  struct protid *newpi = NULL;
+  struct peropen *newpo = NULL;
 
   if (!dircred)
     return EOPNOTSUPP;
@@ -257,8 +257,8 @@ diskfs_S_dir_lookup (struct protid *dirc
              if (! error)
                {
                  error = diskfs_create_protid (newpo, user, &newpi);
-                 if (error)
-                   diskfs_release_peropen (newpo);
+                 if (! error)
+                   newpo = NULL;
                }
 
              iohelp_free_iouser (user);
@@ -269,6 +269,7 @@ diskfs_S_dir_lookup (struct protid *dirc
 
          dirport = ports_get_send_right (newpi);
          ports_port_deref (newpi);
+         newpi = NULL;
          if (np != dnp)
            mutex_unlock (&dnp->lock);
 
@@ -460,28 +461,24 @@ diskfs_S_dir_lookup (struct protid *dirc
                               dircred->po, &newpo);
 
   if (! error)
-    {
-      error = diskfs_create_protid (newpo, dircred->user, &newpi);
-      if (error)
-       diskfs_release_peropen (newpo);
-    }
+    error = diskfs_create_protid (newpo, dircred->user, &newpi);
 
   if (! error)
     {
+      newpo = NULL;
       if (flags & O_EXLOCK)
        error = fshelp_acquire_lock (&np->userlock, &newpi->po->lock_status,
                                     &np->lock, LOCK_EX);
       else if (flags & O_SHLOCK)
        error = fshelp_acquire_lock (&np->userlock, &newpi->po->lock_status,
                                     &np->lock, LOCK_SH);
-      if (error)
-       ports_port_deref (newpi); /* Get rid of NEWPI.  */
     }
 
   if (! error)
     {
       *returned_port = ports_get_right (newpi);
       ports_port_deref (newpi);
+      newpi = NULL;
     }
 
  out:
@@ -495,5 +492,10 @@ diskfs_S_dir_lookup (struct protid *dirc
   if (dnp)
     diskfs_nput (dnp);
 
+  if (newpi)
+    ports_port_deref (newpi);
+  if (newpo)
+    diskfs_release_peropen (newpo);
+
   return error;
 }