linux-kernel - Re: [GIT PULL] block drivers for 2.6.39-rc

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: <20110329131824.GH12760@beardog.cce.hp.com>
Date:	Tue, 29 Mar 2011 08:18:24 -0500
From:	scameron@...rdog.cce.hp.com
To:	Jens Axboe <jaxboe@...ionio.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	scameron@...rdog.cce.hp.com
Subject: Re: [GIT PULL] block drivers for 2.6.39-rc

On Tue, Mar 29, 2011 at 08:34:02AM +0200, Jens Axboe wrote:
> On 2011-03-29 01:06, Linus Torvalds wrote:
> > On Sat, Mar 26, 2011 at 11:56 AM, Jens Axboe <jaxboe@...ionio.com> wrote:
> >> Hi Linus,
> >>
> >> This is the pull request for the block driver updates for 2.6.39. Two
> >> things here:
> >>
> >> - Big drbd update, as per usual...
> >> - cciss update.
> > 
> > Btw, that cciss thing causes a very annoying compiler warning:
> > 
> >   drivers/block/cciss.c: In function ‘dev_show_unique_id’:
> >   drivers/block/cciss.c:617:7: warning: ‘sn[0]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[1]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[2]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[3]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[4]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[5]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[6]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[7]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[8]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[9]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[10]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[11]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[12]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[13]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[14]’ may be used
> > uninitialized in this function
> >   drivers/block/cciss.c:617:7: warning: ‘sn[15]’ may be used
> > uninitialized in this function
> > 
> > which is because the compiler doesn't really notice that those things
> > are only used if the error return isn't being set.
> > 
> > So it's a compiler mis-feature, but the thing is, the warning could
> > easily be avoided by just writing the code more prettily.
> > 
> > Just do the memcpy() unconditionally: we know that 'drv' is a valid
> > pointer (we just loaded 'h' off it), and we're talking about a nice
> > constant 16-byte copy.
> > 
> > IOW, a patch something like the attached.
> > 
> > Untested, but it looks really obvious. Hmm?
> 
> Looks good to me. I'm curious, you get a warning in dev_show_unique_id()
> but not in dev_show_vendor() or any of the other following 4-5 functions
> written in the same style?
> 
> My gcc here does not warn for any of them. But it is very convoluted for
> what it does

Looks ok to me.

FWIW, I've also not seen warnings for any of those from gcc on several
distros (rhel5, rhel6, sles10, sles11).

The convolutedness was probably just me trying to be overly economical
though that "else" after the return and just before the snprintf is
kinda nuts.

-- steve
--
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/