linux-kernel - Re: [PATCH v2] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer

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: <AANLkTin7HaZ5GNnPmM066K+OJwEzCUpw4-oGtAhXSgRt@mail.gmail.com>
Date:	Sun, 30 Jan 2011 15:02:23 -0500
From:	Devin Heitmueller <dheitmueller@...nellabs.com>
To:	Peter Huewe <PeterHuewe@....de>
Cc:	Mauro Carvalho Chehab <mchehab@...radead.org>,
	Julia Lawall <julia@...u.dk>, linux-media@...r.kernel.org,
	linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
	Steven Toth <stoth@...nellabs.com>, Tejun Heo <tj@...nel.org>,
	Dan Carpenter <error27@...il.com>
Subject: Re: [PATCH v2] video/saa7164: Fix sparse warning: Using plain integer
 as NULL pointer

On Sun, Jan 30, 2011 at 2:33 PM, Peter Huewe <PeterHuewe@....de> wrote:
> From: Peter Huewe <peterhuewe@....de>
>
> This patch fixes the warning "Using plain integer as NULL pointer",
> generated by sparse, by replacing
>        if(var == 0)
> with
>        if (!var)
> after an allocation
> and all other offending 0s with NULL.
>
> KernelVersion: linus' tree-1f0324c
>
> Signed-off-by: Peter Huewe <peterhuewe@....de>
> ---
> v2: I changed the patch according to Julia's hints.
> i.e. using if(!buf) instead of if(buf==NULL) after the kmalloc family,
> and other allocations.

Ok, so now we've gone from "eliminating a couple of sparse warnings"
to "going through the rest of the code and jamming in some alternate
coding style when there was nothing wrong with it in the first place."

Don't get me wrong, I appreciate the role of the kernel janitors in
general, but this patch is crap.  It's changes like this that just
lower the signal/noise ratio on *real* work going on, and increasing
the likelihood that some well intentioned janitor screwed up one of
the conversions.

After all, it's not like the author of this patch actually tried the
resulting code.  He only has to mess up one of those two line changes
and we go from "driver that works perfectly well" to "driver that is
100% broken".

There are much better uses of the maintainer's time than going through
patches like this to make sure the submitter didn't screw up the a
conversion that provides no real value.

A change like this:

-       if (buf == NULL) {
+       if (!buf) {

is a worthless change, and there is is a higher chance that one of
them gets screwed up in the patch than what we had to start with.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
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/