public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Jim Rees <rees@umich.edu>
To: Steve Dickson <SteveD@redhat.com>
Cc: Linux NFS Mailing list <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 2/2] nfsidmap: Added -v flag
Date: Thu, 10 Nov 2011 19:23:25 -0500	[thread overview]
Message-ID: <20111111002210.GA2902@umich.edu> (raw)
In-Reply-To: <4EBC4153.9090003@RedHat.com>

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.

  reply	other threads:[~2011-11-11  0:23 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-10 20:26 [PATCH 0/2] nfsidmap: Added error logging and verbosity flag Steve Dickson
2011-11-10 20:26 ` [PATCH 1/2] nfsidmap: Added Error Logging Steve Dickson
2011-11-10 20:26 ` [PATCH 2/2] nfsidmap: Added -v flag Steve Dickson
2011-11-10 21:11   ` Jim Rees
2011-11-10 21:25     ` Steve Dickson
2011-11-11  0:23       ` Jim Rees [this message]
2011-11-11 14:55         ` Steve Dickson
2011-11-10 21:19   ` Bryan Schumaker
2011-11-10 21:27     ` Steve Dickson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111111002210.GA2902@umich.edu \
    --to=rees@umich.edu \
    --cc=SteveD@redhat.com \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox