| Message ID | 20170412083703.11552-4-luc.vanoostenryck@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
I have some very minor comment. On Wed, Apr 12, 2017 at 1:37 AM, Luc Van Oostenryck <luc.vanoostenryck@gmail.com> wrote: > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> > --- > lib.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/lib.c b/lib.c > index 8c9c36bd5..1d21a2fb7 100644 > --- a/lib.c > +++ b/lib.c > @@ -454,6 +454,25 @@ static void handle_arch_finalize(void) > } > > > +static int handle_simple_switch(const char *arg, const char *name, int *flag) > +{ > + int val = 1; > + > + // Prefixe "no-" mean to turn flag off. > + if (strncmp(arg, "no-", 3) == 0) { You don't need to compare to zero. This can be written as: if (!strncmp(arg, "no-", 3)) { That is how most of the strcmp done in sparse. > + if (strcmp(arg, name) == 0) { > + *flag = val; > + return 1; > + } Same here. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 1, 2017 at 8:15 AM, Christopher Li <sparse@chrisli.org> wrote: > I have some very minor comment. > >> +static int handle_simple_switch(const char *arg, const char *name, int *flag) >> +{ >> + int val = 1; >> + >> + // Prefixe "no-" mean to turn flag off. >> + if (strncmp(arg, "no-", 3) == 0) { > > You don't need to compare to zero. This can be written as: > if (!strncmp(arg, "no-", 3)) { > > That is how most of the strcmp done in sparse. I'm not sure exactly what you want me to do in this case. Is it just a comment for things to avoid in the future or would you like me to change it before it is integrated? Of course, I know that it is not needed to compare it against zero, I do it purposely because when I use "if (!x) " it means to me something like "x is invalid" or "x is not true" and my mental model of string compare never match with "is invalid" or "is not true", so I use the "if (x == 0)" idioms to be very clear that for strcmp() a return value of 0 means "the two strings are equal". But I don't mind, I can change it if you prefer. What annoys me is that this code have been posted more than 7 weeks ago, is now part of the pull request I sent 2 weeks ago and by now I have 35 topic branches that depend on this code and I certainly would prefer to do more usefull things than handling these nits and rewrite all my history. -- Luc -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/lib.c b/lib.c index 8c9c36bd5..1d21a2fb7 100644 --- a/lib.c +++ b/lib.c @@ -454,6 +454,25 @@ static void handle_arch_finalize(void) } +static int handle_simple_switch(const char *arg, const char *name, int *flag) +{ + int val = 1; + + // Prefixe "no-" mean to turn flag off. + if (strncmp(arg, "no-", 3) == 0) { + arg += 3; + val = 0; + } + + if (strcmp(arg, name) == 0) { + *flag = val; + return 1; + } + + // not handled + return 0; +} + static char **handle_switch_o(char *arg, char **next) { if (!strcmp (arg, "o")) { // "-o foo"
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- lib.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)