Message-ID: <4EBD3754.6070602@RedHat.com>
Date: Fri, 11 Nov 2011 09:55:16 -0500
From: Steve Dickson <SteveD@redhat.com>
MIME-Version: 1.0
To: Jim Rees <rees@umich.edu>
CC: Linux NFS Mailing list <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 2/2] nfsidmap: Added -v flag
References: <1320956785-18004-1-git-send-email-steved@redhat.com> <1320956785-18004-3-git-send-email-steved@redhat.com> <20111110211117.GA2011@umich.edu> <4EBC4153.9090003@RedHat.com> <20111111002210.GA2902@umich.edu>
In-Reply-To: <20111111002210.GA2902@umich.edu>
Content-Type: text/plain; charset=ISO-8859-1
Sender: linux-nfs-owner@vger.kernel.org


On 11/10/2011 07:23 PM, Jim Rees wrote:
> Steve Dickson wrote:
> 
>   
>   
>   On 11/10/2011 04:11 PM, Jim Rees wrote:
>   > Steve Dickson wrote:
>   > 
>   >   To aid in debugging, the -v flag can now be specified
>   >   on the command to enable verbose logging in both
>   >   the nfsidmap command and libnfsidmap library routines.
>   >   
>   >   Signed-off-by: Steve Dickson <steved@redhat.com>
>   >   ---
>   >    utils/nfsidmap/nfsidmap.c   |   12 ++++++++++++
>   >    utils/nfsidmap/nfsidmap.man |   15 ++++++++++++---
>   >    2 files changed, 24 insertions(+), 3 deletions(-)
>   >   
>   >   diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
>   >   index 134d9bc..d74189a 100644
>   >   --- a/utils/nfsidmap/nfsidmap.c
>   >   +++ b/utils/nfsidmap/nfsidmap.c
>   >   @@ -12,6 +12,7 @@
>   >    #include <syslog.h>
>   >    #include "xlog.h"
>   >    
>   >   +int verbose = 0;
>   >    /* gcc nfsidmap.c -o nfsidmap -l nfsidmap -l keyutils */
>   >    
>   >    #define MAX_ID_LEN   11
>   >   @@ -108,6 +109,12 @@ int main(int argc, char **argv)
>   >    	xlog_syslog(1);
>   >    	xlog_stderr(0);
>   >    
>   >   +	if (argc > 1 && strcmp(argv[1], "-v") == 0) {
>   >   +		verbose = 1;
>   >   +		nfs4_set_debug(1, NULL);
>   >   +		argc--, argv++;
>   >   +	}
>   >   +
>   > 
>   > Ugh.  Is there some reason not to use getopt() like all the other utils do?
>   I was waiting for this comment ;-)
> 
> I'm always happy to play your straight man.
> 
>   I decided to go with how the command was originally written
>   because this command is only call from the kernel so a user
>   should execute it (except for debugging).  
>   
>   The arguments are vary static on where they need to be
>   no command line. So its either going to work or not,
>   which means there is no real need for a usage error (expect
>   for the one I added). 
>   
>   Finally, is there real need for a while loop and switch statement 
>   for on simple case? I thought not...
> 
> It's more work for the next guy who comes along and wants to add another
> option, especially if the new option takes an argument.
I still think its overkill but I'm always a fan 
of making easier for the next guys down the line...
I'll re-spin it... 

steved.