Re: [PATCH 03/17] mount.nfs: Remove support for "-t" option

[Chuck Lever <chuck.lever@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 2223 bytes --]

Neil Brown wrote:
> On Sunday July 15, chuck.lever@oracle.com wrote:
>> /bin/mount will never pass "-t" to a mount helper, since it passes the
>> fs-type in the name of the program it is executing.
> 
> Does it hurt to leave it there though?  Maybe it makes unit-testing
> easier?
> Or maybe not....

Well, part of these changes remove stuff that just seems confusing to 
anyone trying to understand the code, and yes, this one is quite 
optional.  Honestly, I couldn't think of a case where this could 
possibly be useful, but I'm open to education.

The fact that we can do a "mount.nfs -t nfs4" and that's the same as 
"mount.nfs4" seems a little weird.  The mount command will parse the 
"-t" option and invoke either "mount.nfs" or "mount.nfs4"... extra logic 
here is just going to cloud things.

>> @@ -413,23 +413,12 @@ int main(int argc, char *argv[])
>>  	mount_point = argv[2];
>>  
>>  	argv[2] = argv[0]; /* so that getopt error messages are correct */
>> -	while ((c = getopt_long (argc - 2, argv + 2, "rt:vVwfno:hs",
>> +	while ((c = getopt_long (argc - 2, argv + 2, "r:vVwfno:hs",
>                                                       ^^
> 
> Ooops.  -r takes an argument now?

Hmmm.  I'll fix that too.

>>  		if ((mc = getfsfile(mount_point)) == NULL ||
>> -		    strcmp(mc->m.mnt_fsname, spec) != 0 ||
>> -		    strcmp(mc->m.mnt_type, (nfs_mount_vers == 4 ? "nfs4":"nfs")) != 0
>> -		    ) {
>> -			fprintf(stderr, "%s: permission died - no match for fstab\n",
>> -				progname);
>> +		    !strcmp(mc->m.mnt_fsname, spec) ||
>                    ^^^^
> 
> I absolutely LOATHE that construct. The first thing you read is "!"
> which means "not" but it is actually a test for "are these things
> equal" which makes it confusing (to me).  So I always like to see the
> result of strcmp tested with "== 0"  "< 0"  "> 0"  as the relation
> given applies exactly to the tested relation between the strings.

OK, now that I know that's the preference, I will use "== 0".

> Further:
>> -		    strcmp(mc->m.mnt_fsname, spec) != 0 ||
> and
>> +		    !strcmp(mc->m.mnt_fsname, spec) ||
> 
> are opposite tests, and it is not clear to me why you inverted the
> test.

Simple explanation: because I'm a bone-head.

[-- Attachment #2: chuck.lever.vcf --]
[-- Type: text/x-vcard, Size: 290 bytes --]

begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture: Linux Projects Group
adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
url:http://oss.oracle.com/~cel
version:2.1
end:vcard


[-- Attachment #3: Type: text/plain, Size: 286 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[-- Attachment #4: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs