[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: Remove use of strcmp, etc



Jim Meyering <jim meyering net> wrote:
> "Daniel P. Berrange" <berrange redhat com> wrote:
>> On Fri, May 09, 2008 at 10:42:08PM +0100, Daniel P. Berrange wrote:
>>> On Fri, May 09, 2008 at 11:40:39PM +0200, Jim Meyering wrote:
>>> > "Daniel P. Berrange" <berrange redhat com> wrote:
>>> >
>>> > > This patch removes all use of strcmp, strncmp, strcasecmp and strncasecmp
>>> > > in favour of the equality macros we have defined in internal.h, eg STREQ,
>>> > > STRNEQ, STRNEQLEN, STREQLEN, etc, etc
>>> >
>>> > Nice.  With that, you can remove sc_prohibit_strcmp
>>> > from the list of disabled checks in Makefile.cfg.
>>> > You might want to extend the corresponding regexp in Makefile.maint
>>> > to prohibit the other completely excluded functions like strncmp.
>>> >
>>> > BTW, did you make this change automatically?  or do a binary before/after
>>> > comparison.  In which case there's not much point in reviewing the details...
>>>
>>> No I did it manually & checked with inspection. Could try a binary diff
>>> with & without the patch if you want...
>>
>> Actually a binary diff won't work because of places where I changed
>> from strncmp to  STRPREFIX - the latter includes a calls to strlen
>> instead of hardcoding the constant
>
> To check, so far I've converted all strcmp uses with these:
>
> git grep -l '!strcmp *('|xargs perl -pi -e 's/!strcmp( *)\(/STREQ$1(/g'
> git grep -l 'strcmp *([^=]*== *0'|xargs perl -pi -e 's/\bstrcmp( *\(.*?\)) *== *0/STREQ$1/'
> git grep -l 'strcmp *([^=]*!= *0'|xargs perl -pi -e 's/\bstrcmp( *\(.*?\)) *!= *0/STRNEQ$1/g'
>
> git grep -l '!(strcmp *('|xargs perl -pi -e 's/!\(strcmp( *\(.*?\))\)/STREQ$1/'
>
> git grep -l 'strcmp *('|xargs perl -pi -e 's/\bstrcmp( *\()/STRNEQ$1/g'
>
> Next (later ;-) is to convert strncmp uses and compare with yours.

ACK.

I finished the job with the following.
That exposed another binary difference (which is fine):

-    if (STRNEQ(str, "linux"))
+    if (STREQ(str, "hvm"))
         hvm = 1;

I automatically changed most uses of STRN?EQLEN (e.g.,
-        STRNEQLEN((const char *)target, "hd", 2) &&
+        !STRPREFIX((const char *)target, "hd") &&
to uses of STRPREFIX.  That matched all of yours and caught a few more,
so you might want to run the last two conversions to keep them all
together.

# Change only .[ch] files, and not internal.h.
f() { grep '\.[ch]$'|grep -vE 'gnulib/|internal\.h'; }

git grep -l '!strncmp *('|grep '\.[ch]$' |f|xargs \
  perl -pi -e 's/!strncmp( *)\(/STREQLEN$1(/g'

# strncmp(...) != 0 -> STRNEQLEN(...)
git grep -l 'strncmp *([^=]*!= *0'|f|xargs \
  perl -pi -e 's/\bstrncmp( *\(.*?\)) *!= *0/STRNEQLEN$1/g'

# !strcasecmp -> STRCASEEQ
git grep -l '!strcasecmp *('|f|xargs \
  perl -pi -e 's/!strcasecmp( *)\(/STRCASEEQ$1(/g'

# strcasecmp() == 0 -> STRCASEEQ
git grep -l 'strcasecmp *([^=]*== *0'| f | xargs \
  perl -pi -e 's/\bstrcasecmp( *\(.*?\)) *== *0/STRCASEEQ$1/'

# strncmp(...) == 0 -> STREQLEN(...)
git grep -l 'strncmp *([^=]*== *0'| f | xargs \
  perl -pi -e 's/\bstrncmp( *\(.*?\)) *== *0/STREQLEN$1/'

# strncmp(...) -> STRNEQLEN(...)
git grep -l 'strncmp *('|f|xargs perl -pi -e 's/\bstrncmp( *\()/STRNEQLEN$1/g'

# strcasecmp() != 0 -> STRCASENEQ
git grep -l 'strcasecmp *([^=]*!= *0'| f | xargs \
  perl -pi -e 's/\bstrcasecmp( *\(.*?\)) *!= *0/STRCASENEQ$1/'

# Do this:
# -        STRNEQLEN((const char *)target, "hd", 2) &&
# +        !STRPREFIX((const char *)target, "hd") &&
git grep -l '\<STRNEQLEN *('| f | xargs \
  perl -pi -e 's/\bSTRNEQLEN( *\(.*?, *".*?"), \d+\)/!STRPREFIX$1)/'

# Do this:
# -                } else if (STREQLEN(key, "vnclisten=", 10)) {
# +                } else if (STRPREFIX(key, "vnclisten=")) {
git grep -l '\<STREQLEN *('| f | xargs \
  perl -pi -e 's/\bSTREQLEN( *\(.*?, *".*?"), \d+\)/STRPREFIX$1)/'

# Two missed by the above
# src/qemu_conf.c:  if (STREQLEN("vnet", (const char*)ifname, 4)) {
# src/xm_internal.c: if (STREQLEN(ent->d_name, QEMU_IF_SCRIPT, strlen(QEMU_IF_SCRIPT)))
# Since there are 3 more like the latter, do them automatically:
git grep -l '\<STREQLEN *('| f | xargs \
  perl -pi -e 's/\bSTREQLEN( *\(.*?, *(\w+)), strlen\(\2\)\)/STRPREFIX$1)/'


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]