[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: prohibit_strcmp
From: |
Jim Meyering |
Subject: |
Re: prohibit_strcmp |
Date: |
Thu, 23 Feb 2012 17:52:53 +0100 |
Akim Demaille wrote:
> Bison features some warnings from syntax-check:
>
> prohibit_strcmp
> ../../doc/bison.texinfo:2572: if (strcmp (ptr->name,sym_name) == 0)
> ../../src/files.c:145: if (strcmp (ext, ".y") == 0)
> ../../src/muscle-tab.c:53: return strcmp (m1->key, m2->key) == 0;
> ../../src/muscle-tab.c:410: if (!strcmp (conversion[i].obsolete, variable))
> ../../src/print_graph.c:136: && strcmp (symbols[sym]->tag, "error") !=
> 0)
> ../../src/uniqstr.c:114: return strcmp (m1, m2) == 0;
> maint.mk: replace strcmp calls above with STREQ/STRNEQ
>
> I don't understand too well what is expected from me here.
> While I see in
>
> ../../src/files.c:145: if (strcmp (ext, ".y") == 0)
> ../../src/print_graph.c:136: && strcmp (symbols[sym]->tag, "error") !=
> 0)
Hi Akim,
> an opportunity to write a static pattern instead of the constant
> string, in the remaining cases, I doubt there's really value to
> move to streq's STREQ. Actually, in gnulib itself, there are quite many
> strcmp, including with literal strings, and all the STREQ/STRNEQ
> are really using literal strings.
>
> So maybe this warning refers to another kind of STREQ/NEQ? Such as:
>
> fnmatch.c:# define STREQ(s1, s2) (strcmp (s1, s2) == 0)
> fnmatch.c: (STREQ (string, "alpha") || STREQ (string, "upper") \
> fnmatch.c: || STREQ (string, "lower") || STREQ (string, "digit") \
> fnmatch.c: || STREQ (string, "alnum") || STREQ (string, "xdigit") \
> fnmatch.c: || STREQ (string, "space") || STREQ (string, "print") \
> fnmatch.c: || STREQ (string, "punct") || STREQ (string, "graph") \
> fnmatch.c: || STREQ (string, "cntrl") || STREQ (string, "blank"))
Those are in a file that's nominally maintained in glibc,
so we cannot make such stylistic changes to it here in gnulib.
That gnulib has two definitions is unfortunate, but I will not
volunteer to change the STREQ that I've been using ;-)
I don't see much value in streq.h, either, and hence, don't use it.
In the same vein, it's good to avoid strncmp calls.
When comparing against a literal (and with no trailing NULL),
I use STRNCMP_LIT defined in coreutils/src/system.h:
/* Just like strncmp, but the second argument must be a literal string
and you don't specify the length. */
#define STRNCMP_LIT(s, literal) \
strncmp (s, "" literal "", sizeof (literal) - 1)
> which, I concur, is more readable. It is confusing that there are
> two sets of STREQ in gnulib, and at least the comment for prohibit_strcmp
> should point this out.
The definition I intended to suggest via that syntax-check rule is
short enough that we could even include it in the diagnostic, I suppose.
#define STREQ(a, b) (strcmp (a, b) == 0)