From: Andrew Morton <akpm@linux-foundation.org>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: adam radford <aradford@gmail.com>,
linux-scsi@vger.kernel.org, rdunlap@xenotime.net,
megaraidlinux@lsi.com, viro@zeniv.linux.org.uk
Subject: Re: [patch 2/6] drivers/scsi/megaraid.c: fix sparse warnings
Date: Tue, 10 Jan 2012 17:48:14 -0800 [thread overview]
Message-ID: <20120110174814.351384c1.akpm@linux-foundation.org> (raw)
In-Reply-To: <1326245138.3264.121.camel@dabdike.int.hansenpartnership.com>
On Tue, 10 Jan 2012 20:25:38 -0500 James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> > Since I have no HP firmware on one of these boards, and I can see that
> > altering the firmware string will alter the behvior of the above code
> > WRT setting/clearing BOARD_64BIT in the adapter->flag field, I have to
> > NACK this patch. I hope we can live with the sparse warning since I
> > have no way to test this.
>
> So I think the title of the patch is misleading.
"fix sparse warning" is almost always a bad or wrong patch title.
Sparse warns about "X", and what we're fixing is "X", not the warning.
> The point is that
>
> adapter->product_info.fw_version[] >> 8
>
> Always produces zero since adapter->product_info.fw_version is a char[].
> It seems that the idea was
>
> If you want to supply an update that just making the zero explicit for
> the firmware version (and thus fixes the sparse warning) is fine too ...
> Al just thought that you meant to take the firmware version as BCD
> nybbles.
Yes, it sounds like this is a day-one bug which we can't really fix
now, with an acceptable level of effort or risk. I agree that replacing
it with literal zero and a decent code comment is appropriate.
next prev parent reply other threads:[~2012-01-11 1:43 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-10 23:42 [patch 2/6] drivers/scsi/megaraid.c: fix sparse warnings akpm
2012-01-11 1:18 ` adam radford
2012-01-11 1:25 ` James Bottomley
2012-01-11 1:48 ` Andrew Morton [this message]
2012-01-11 2:07 ` adam radford
-- strict thread matches above, loose matches on Subject: below --
2011-11-15 22:58 akpm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120110174814.351384c1.akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=James.Bottomley@HansenPartnership.com \
--cc=aradford@gmail.com \
--cc=linux-scsi@vger.kernel.org \
--cc=megaraidlinux@lsi.com \
--cc=rdunlap@xenotime.net \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox