Re: [libvirt] [PATCH] cfg.mk: Introduce rule for setlocale()
On 06/04/2014 10:24 AM, Michal Privoznik wrote:
>
>>> +# Require #include <locale.h> in all files that call setlocale()
>>> +sc_require_locale_h:
>>> + @for i in $$($(VC_LIST_EXCEPT) | grep '\.[chx]$$');
>>> do \
>>> + if test -z "$$(grep setlocale $$i)" ; then continue; fi;
\
>>
>> Why not:
>>
>> if ! grep -q setlocale $$i; then continue; fi
>
> Even simpler, let maint.mk do it for you:
>
> sc_require_locale_h:
> @require='include.*locale\.h' \
> containing='setlocale *(' \
> halt='setlocale() requires <locale.h>' \
> $(_sc_search_regexp)
>
This is the maint.mk-ism I was looking for. Feel free to replace my
code. Just out of pure curiosity - is your code any faster?
Yes. Your code spawns 1 or 2 greps per file (one spawn per iteration of
the forloop); while maint.mk's rule does tricks like:
: Filter by content; \
test -n "$$files" && test -n "$$containing"
\
&& { files=$$(grep -l "$$containing" $$files); } || :;
\
that use a single grep process to drastically filter the set of files
that then need to be tested.
I see you already pushed, so I'll take your message as the ACK to go
ahead and push my replacement.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)