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)