Re: [libvirt] [PATCH] Fixed NULL pointer check
On 03/19/2012 06:43 AM, Daniel P. Berrange wrote:
>> That is a historical remant. Do feel free to send another
followup patch to
>> cleanup all the cases of 'return(NULL)' as well.
>>
>> Daniel
>
> Did you mean in that file or globally? Because I just tried the first
> thing that came to my mind and look at the output:
>
> libvirt $ find . -name '*.[ch]' -type f -exec grep -nH -e \
> 'return(.*);' {} + | wc -l
> 852
Yes, it is a rather big cleanup, which is all the more reason why we'd
want to ensure that 'make syntax-check' prevents it from recurring.
>
> Not that I wouldn't do it, it just seem as a pretty big change. On the
> other hand, I don't see the point in changing that in only one file.
IMHO, we should do this to make our code consistent, and ideally add a
syntax-check rule to prevent them coming back. Anyone disagree ?
I'm in favor of such a cleanup. There are still a few places where
return needs parenthesis; for example, I'm fond of the style:
return (big_long_conditional ?
option_1 :
option_2);
since the open '(' lets the rest of the code indent nicely when using
default emacs indentation. But it's still pretty easy to recognize the
difference between complex returns and the real offenders. I think the
number of false positives and false negatives is pretty near zero if you
boil it down to detecting uses where there are no spaces between the '('
and ')'. Thus, for cfg.mk, I suggest a pattern something like:
sc_prohibit_return_as_function:
@prohibit='\<return *([^ ]*)' \
halt='avoid extra () with return statements' \
$(_sc_search_regexp)
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)