Re: [PATCH] udf: fix uid/gid options and add uid/gid=ignore and forget options

[Phillip Susi <psusi@cfl.rr.com>]

Pekka J Enberg wrote:
> Hi Phillip,
>
> On Sun, 5 Mar 2006, Phillip Susi wrote:
> > My bad, I meant to remove those ifs, not just prepend them with an else.  Try
> > this one:
> >
> > c8393f6e4fe6159fd916f3c68091e76bbfdc5fd8
> > diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> > index 395e582..49aeac3 100644
> > --- a/fs/udf/inode.c
> > +++ b/fs/udf/inode.c
> > @@ -1045,10 +1045,12 @@ static void udf_fill_inode(struct inode        }
> >
> >        inode->i_uid = le32_to_cpu(fe->uid);
> > -      if ( inode->i_uid == -1 ) inode->i_uid = UDF_SB(inode->i_sb)->s_uid;
> > +      if ( inode->i_uid == -1 || UDF_QUERY_FLAG(inode->i_sb,
> > UDF_FLAG_UID_IGNORE) )
> > +        inode->i_uid = UDF_SB(inode->i_sb)->s_uid;
>
> Formatting.

What exactly do you mean by formatting?

> >        inode->i_gid = le32_to_cpu(fe->gid);
> > -      if ( inode->i_gid == -1 ) inode->i_gid = UDF_SB(inode->i_sb)->s_gid;
> > +      if ( inode->i_gid == -1 || UDF_QUERY_FLAG(inode->i_sb,
> > UDF_FLAG_GID_IGNORE) )
> > +        inode->i_gid = UDF_SB(inode->i_sb)->s_gid;
>
> Same here.
>
> >        inode->i_nlink = le16_to_cpu(fe->fileLinkCount);
> >        if (!inode->i_nlink)
> > @@ -1335,11 +1337,13 @@ udf_update_inode(struct inode *inode, in
> >        return err;
> >        }
> >
> > -      if (inode->i_uid != UDF_SB(inode->i_sb)->s_uid)
> > -      fe->uid = cpu_to_le32(inode->i_uid);
> > -
> > -      if (inode->i_gid != UDF_SB(inode->i_sb)->s_gid)
> > -      fe->gid = cpu_to_le32(inode->i_gid);
> > +      if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_UID_FORGET))
> > +        fe->uid = cpu_to_le32(-1);
> > +      else fe->uid = cpu_to_le32(inode->i_uid);
>
> This is better, but if id was -1 on disk, we're overriding it unless the 
> forget mount option was specified. Do we want that? I think my patch is a 
> better fix (if it works anyway) and yours should be on top of that. Did 
> you have the chance to test it?

Yes, if you chown a file that is owned by -1 on disk, you do want it to 
be saved back with the new id, unless you set the forget option.

> Also, formatting is, wrong, just make it
>
> 	if (forget)
> 		fe->uid = ...;
> 	else
> 		fe->uid = ...;

You mean use hard tabstops instead of two spaces to indent?  Is there a 
way to set emacs to do that?  It automatically uses the two spaces.

> > +      if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_GID_FORGET))
> > +        fe->gid = cpu_to_le32(-1);
> > +      else fe->gid = cpu_to_le32(inode->i_gid);
> >
> >        udfperms =      ((inode->i_mode & S_IRWXO)     ) |
> >        ((inode->i_mode & S_IRWXG) << 2) |
>
> Same here.
>
> Please document the new mount options in Documentation/filesystems/udf.txt.

Working on it.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/