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)/'