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.