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)