On Wed, 2009-11-18 at 11:07 -0700, David Brownell wrote:
> On Wednesday 18 November 2009, Zachary T Welch wrote:
> > Adds several macros similar to COMMAND_PARSE_NUMBER, but for parsing
> > boolean command arguments.  Two flavors are provided to provide
> > drop-in compatibility with existing code, allow for the elimination
> > of a lot of code bloat while improving the error checking and reporting.
> > 
> > COMMAND_PARSE_ON_OFF parses "on"/"off" command parameters.
> > COMMAND_PARSE_ON_OFF parses "enable"/"disable" command parameters.
> 
> Same function, different behavior?  Or cut/paste error?  :)

Copy/paste.  :)

> It's actually fairly common to have "bool" parsing code
> handle multiple variants, and I suggest you do that here
> instead of defining "friends".

I preserved backward compatibility.

> Variants could be:  different case (call strcasecmp),
> numeric mode (0/1), on/off, enable/disable, true/false,
> yes/no.

Did you look at patch 4?  All but one of these options is now in place
(yes/no), and a small patch can switch over all of the new call points
to use the single macro.  I was planning ahead for this eventuality,
though I could make this part of the current series.

> It gets annoying to need to recall which variant of
> true/false a given routine needs.  :)

It's part of a generally good rule too: be forgiving with what you
accept for input, and strict with what you emit for output.

> 
> > 
> > Both print the error and return an error out of the calling function.
> > 
> > Signed-off-by: Zachary T Welch <[email protected]>
> > ---
> >  src/helper/command.h |   28 ++++++++++++++++++++++++++++
> >  1 files changed, 28 insertions(+), 0 deletions(-)
> > 
> > diff --git a/src/helper/command.h b/src/helper/command.h
> > index 05088b5..9f2d971 100644
> > --- a/src/helper/command.h
> > +++ b/src/helper/command.h
> > @@ -263,6 +263,34 @@ DECLARE_PARSE_WRAPPER(_s8, int8_t);
> >             } \
> >     } while (0)
> >  
> > +/**
> > + * Parse the string @c as a binary parameter, storing the boolean value
> > + * in @c out.  The strings @c on and @c off are used to match different
> > + * strings for true and false options (e.g. "on" and "off" or
> > + * "enable" and "disable").
> > + */
> > +#define COMMAND_PARSE_BOOL(in, out, on, off) \
> 
> This is quite large for an inline.  Better as a function.
> Especially if you do a way with the "friends"...
> 
> 
> > +   do { \
> > +           if (strcmp(in, on) == 0) \
> > +                   out = true; \
> > +           else if (strcmp(in, off) == 0) \
> > +                   out = false; \
> > +           else { \
> > +                   command_print(CMD_CTX, stringify(out) \
> > +                           " option value ('%s') is not valid", in); \
> > +                   command_print(CMD_CTX, "  choices are '%s' or '%s'", \
> > +                           on, off); \
> > +                   return ERROR_COMMAND_SYNTAX_ERROR; \
> > +           } \
> > +   } while (0)
> > +
> > +/// parses an on/off command argument
> > +#define COMMAND_PARSE_ON_OFF(in, out) \
> > +           COMMAND_PARSE_BOOL(in, out, "on", "off")
> > +/// parses an enable/disable command argument
> > +#define COMMAND_PARSE_ENABLE(in, out) \
> > +           COMMAND_PARSE_BOOL(in, out, "enable", "disable")
> > +
> >  void script_debug(Jim_Interp *interp, const char *cmd,
> >             unsigned argc, Jim_Obj *const *argv);
> >  
> > -- 
> > 1.6.4.4
> > 
> > _______________________________________________
> > Openocd-development mailing list
> > [email protected]
> > https://lists.berlios.de/mailman/listinfo/openocd-development
> > 
> > 
> 


_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to