bug-gnulib
[Top][All Lists]
Advanced

[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)



reply via email to

[Prev in Thread] Current Thread [Next in Thread]