[linux-cifs-client] Re: patch to mount.cifs to allow it to handle USER env variable

[Steve French]

On Thu, Dec 4, 2008 at 10:46 AM, Jeff Layton <jlayton at redhat.com> wrote:
> On Thu, 4 Dec 2008 10:22:21 -0600
> "Steve French" <smfrench at gmail.com> wrote:
>
>> Although the USER env variable is usually set to the same thing as
>> getuid returns, the user may want to override it (and mount.cifs man
>> page, like smbfs, says it is supposed to honor the ENV variable $USER.
>>  Patch follows to fix that:
>>
>> diff --git a/source/client/mount.cifs.c b/source/client/mount.cifs.c
>> index 3b56e5f..8e4d521 100644
>> --- a/source/client/mount.cifs.c
>> +++ b/source/client/mount.cifs.c
>> @@ -1269,7 +1269,13 @@ int main(int argc, char ** argv)
>>         }
>>
>>         if(got_user == 0) {
>> -               user_name = getusername();
>> +               /* Note that the password will not be retrieved from the
>> +                  USER env variable (ie user%password form) as there is
>> +                  already a PASSWD environment varaible */
>> +               if (getenv("USER"))
>> +                       user_name = strdup(getenv("USER"));
>> +               if (user_name == NULL)
>> +                       user_name = getusername();
>>                 got_user = 1;
>>         }
>>
>
> Why not put all of that logic in getusername() instead? main() is
> awfully big, and it would be nice to put that functionality in
> the helper function instead.
>
> Also, there are 2 call sites for getusername() in the sources I'm
> looking at. If you put the new code in getusername() itself then
> the second site gets fixed for "free"...

The main reason that I did not put it in getusername is that the
two calls to getusername appear to be for different purposes,
and only one of those two makes sense for the $USER override.

getusername is invoked when all other ways of finding the username
to use during smb session setup fails
(ie it is not specified on mount, it is not in a credential file,
and (at least with my patch) it is not in $USER environment variable)

The second use of getusername is to update the mount table after
mount succeeds to indicate who performed the mount (the username
for the uid of the mounter).  If we added the genenv call inside
getusername we could show a name that is different from the local user
(perhaps a non-existent user) in the mount table.  When you specify
the user via $USER you would get the smb user, but when you specify
a credential file or pass "user=" on the mount command you would get
a different user in the mount table if we made the change you suggest.
I don't know if there is a good reason to put the local user in the mount
table (instead of the smb user) but we should be consistent (if we are going
to put the smb user we shouldn't use getusername, but instead use
the "user_name" variable
local user)
-- 
Thanks,

Steve