Re: RFC: [PATCH] trans/fakeroot.c

[Svante Signell]

On Tue, 2015-06-09 at 14:20 +0200, Samuel Thibault wrote:
> Svante Signell, le Tue 09 Jun 2015 11:41:01 +0200, a écrit :
> > On Tue, 2015-06-09 at 11:11 +0200, Samuel Thibault wrote:
> > 
> > > So the package is actually doing something stupid (yes, that's what you
> > > should have written in your mail to explain what is happening, actually
> > > :) ). But it does work as root as specified by POSIX, so we have to
> > > support it.
> > 
> > Yes I think there is a package bug (it's up to the package maintainer to
> > write good code, not me pointing fingers):
> 
> Well, the code is supposed to be run by root, so it's actually sorta
> "correct" :)

The difference is that real root can change the directory mode to
drw-r--r-- and still access everything below, but fakeroot has to do at
least drwxr--r- for anybody except root itself to read that directory.

> You also need to fix the indentation.

Fixed :)

> > +    if ((real_mode & S_IFDIR) 
> 
> I don't think it's so simple, see the precise S_IFMT bit matter. That
> should most probably be rather an S_ISDIR call.

New patch attached using the IS_ISDIR macro. Better now?

Index: hurd-0.6.git20150523/trans/fakeroot.c
===================================================================
--- hurd-0.6.git20150523.orig/trans/fakeroot.c
+++ hurd-0.6.git20150523/trans/fakeroot.c
@@ -548,7 +548,6 @@ real_from_fake_mode (mode_t mode)
 error_t
 netfs_attempt_chmod (struct iouser *cred, struct node *np, mode_t mode)
 {
-  struct netnode *nn;
   mode_t real_mode;
 
   if ((mode & S_IFMT) == 0)
@@ -558,18 +557,13 @@ netfs_attempt_chmod (struct iouser *cred
 
   /* Make sure that `check_openmodes' will still always be able to reopen
      it.  */
-  real_mode = mode;
-  nn = netfs_node_netnode (np);
-  if (nn->openmodes & O_READ)
-    real_mode |= S_IRUSR;
-  if (nn->openmodes & O_WRITE)
-    real_mode |= S_IWUSR;
-  if (nn->openmodes & O_EXEC)
-    real_mode |= S_IXUSR;
+  real_mode = mode | S_IRUSR | S_IWUSR;
+  if (S_ISDIR (mode))
+      real_mode |= S_IXUSR;
 
   /* We don't bother with error checking since the fake mode change should
      always succeed--worst case a later open will get EACCES.  */
-  (void) file_chmod (nn->file, real_mode);
+  (void) file_chmod (netfs_node_netnode (np)->file, real_mode);
   set_faked_attribute (np, FAKE_MODE);
   np->nn_stat.st_mode = mode;
   return 0;