Thanks.
FYI, recently in a nearby mailing list, we had this gem:
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH 3/5] isdn: fix integer as NULL pointer warning
Date: Fri, 23 May 2008 08:08:57 -0700 (PDT)
Message-ID: <alpine.LFD.1.10.0805230803290.3081@xxxxxxxxxxxxxxxxxxxxxxxxxx>
...
> len += sprintf(page+len, "%-16s %s\n", "type", s);
> - if ((s = cinfo->version[VER_DRIVER]) != 0)
> + if ((s = cinfo->version[VER_DRIVER]) != NULL)
> len += sprintf(page+len, "%-16s %s\n", "ver_driver", s);
For thigns like this (ie testing an assignment), I personally much prefer
s = cinfo->version[VER_DRIVER];
if (s)
len += sprintf(page+len, "%-16s %s\n", "ver_driver", s);
over the uglier and unreadable version.
IOW, testing assignments is good only when:
- you have to do it because of syntax (ie notably in a "while()" loop)
- there's some reason you want it to be a single statement (eg doing a
macro or other thing)
- of the assignment is really simple, and the test is not against NULL or
zero.
The reason for that "the test is not against NULL or zero" is that testing
for NULL and 0 is better done with just a "if (x)", and in an assignment
that just means either (a) a incomprehensible extra parenthesis just to
shut the compiler up or (b) changing the simple test into a stupid test
(ie doing "if (x != NULL)").
(b) is much preferable to (a), but just doing it as two statements is
much preferable to either!
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
