linux-kernel - Re: [PATCH 3/5] isdn: fix integer as NULL pointer warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <alpine.LFD.1.10.0805230803290.3081@woody.linux-foundation.org>
Date:	Fri, 23 May 2008 08:08:57 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Harvey Harrison <harvey.harrison@...il.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/5] isdn: fix integer as NULL pointer warning



On Thu, 22 May 2008, Harvey Harrison wrote:
>  	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!

		Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/