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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 09/04/2014 04:12 AM, Mauro Carvalho Chehab wrote:
Em Thu, 04 Sep 2014 02:56:19 +0300
Antti Palosaari <crope@xxxxxx> 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@xxxxxxxxxxx>

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/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux