Re: [PATCH 32/46] [media] e4000: simplify boolean tests

[Antti Palosaari <crope@iki.fi>]

On 09/04/2014 04:12 AM, Mauro Carvalho Chehab wrote:
> Em Thu, 04 Sep 2014 02:56:19 +0300
> Antti Palosaari <crope@iki.fi> escreveu:
>
>> Which is static analyzer you are referring to fix these?
>
> Coccinelle. See: scripts/coccinelle/misc/boolinit.cocci
>
>> Using true/false for boolean datatype sounds OK, but personally I
>> dislike use of negation operator. For my eyes (foo = 0) / (foo == false)
>> is better and I have changed all the time negate operators to equal
>> operators from my drivers.
>
> The usage of the negation operator on such tests is there since
> the beginning of C.

ugh! :(

>
> By being shorter, a reviewer can read it faster and, at least for
> me, it is a non-brain to understand !foo. On the other hand,
> "false" is not part of standard C. So, it takes more time for my
> brain to parse it.

No, it just opposite for me and many others.

>
> Anyway, from my side, the real reasone for using it is not due to
> that. It is that I (and other Kernel developers) run from time to
> time static analyzers like smatch and coccinelle, in order to identify
> real errors. Having a less-polluted log helps to identify the newer
> errors/warnings.

Have you ever looked Documentation/CodingStyle ?
How about that example, from CodingStyle:
int fun(int a)
{
	int result = 0;
	char *buffer = kmalloc(SIZE);

	if (buffer == NULL)
		return -ENOMEM;

	if (condition1) {
		while (loop1) {
			...
		}
		result = 1;
		goto out;
	}
	...
out:
	kfree(buffer);
	return result;
}

As it shows, it is (buffer == NULL) *not* (!buffer). And if you like to 
do it differently then update CodingStyle first! Add clear mention that 
it should be (!buffer) and change given example to match your style. 
Otherwise, I am happy to do as CodingStyle shows!

Antti

>
> Regards,
> Mauro
>>
>> regards
>> Antti
>>
>> On 09/03/2014 11:33 PM, Mauro Carvalho Chehab wrote:
>>> Instead of using if (foo == false), just use
>>> if (!foo).
>>>
>>> That allows a faster mental parsing when analyzing the
>>> code.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
>>>
>>> diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
>>> index 90d93348f20c..cd9cf643f602 100644
>>> --- a/drivers/media/tuners/e4000.c
>>> +++ b/drivers/media/tuners/e4000.c
>>> @@ -400,7 +400,7 @@ static int e4000_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
>>>    	struct e4000 *s = container_of(ctrl->handler, struct e4000, hdl);
>>>    	int ret;
>>>
>>> -	if (s->active == false)
>>> +	if (!s->active)
>>>    		return 0;
>>>
>>>    	switch (ctrl->id) {
>>> @@ -423,7 +423,7 @@ static int e4000_s_ctrl(struct v4l2_ctrl *ctrl)
>>>    	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>>>    	int ret;
>>>
>>> -	if (s->active == false)
>>> +	if (!s->active)
>>>    		return 0;
>>>
>>>    	switch (ctrl->id) {
>>>
>>

-- 
http://palosaari.fi/