Re: [libvirt] PATCH: Remove use of strcmp, etc
Jim Meyering <jim(a)meyering.net> wrote:
"Daniel P. Berrange" <berrange(a)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(a)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)/'