From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Don Brace <brace77070@gmail.com>
Cc: Joe Handzik <joseph.t.handzik@hp.com>,
"James E.J. Bottomley" <JBottomley@odin.com>,
Kevin Barnett <kevin.barnett@pmcs.com>,
Scott Teel <scott.teel@pmcs.com>, Tomas Henzl <thenzl@redhat.com>,
iss_storagedev@hp.com, storagedev@pmcs.com,
linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] scsi: hpsa: fix multiple issues in path_info_show
Date: Sat, 14 Nov 2015 16:24:24 +0100 [thread overview]
Message-ID: <87mvuga9jr.fsf@rasmusvillemoes.dk> (raw)
In-Reply-To: <56313105.3050805@pmcs.com> (Don Brace's message of "Wed, 28 Oct 2015 15:33:09 -0500")
On Wed, Oct 28 2015, Don Brace <brace77070@gmail.com> wrote:
> On 10/27/2015 05:16 PM, Rasmus Villemoes wrote:
>> I'm not familiar with this code, but path_info_show() (added in
>> 8270b86243658 "hpsa: add sysfs entry path_info to show box and
>> bay information") seems to be broken in multiple ways.
>>
[snip]
>>
>> We can fix all of that and get rid of the 400 byte stack buffer by
>> simply writing directly to the given output buffer, which the upper
>> layer guarantees is at least PAGE_SIZE. s[c]nprintf doesn't care where
>> it is writing to, so this doesn't make the spin lock hold time any
>> longer. Using scnprintf ensures that output_len always represents the
>> number of bytes actually written to the buffer, so we'll report the
>> proper amount to the upper layer.
>>
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>>
> Thanks, I added this to my current patch set. This patch will be up
> with you as the author soon.
I see it in mainline now. May I ask why 6 out of 7 scnprintfs were
changed (back) to snprintf? I don't think there's any functional
difference as long as PAGE_SIZE is indeed sufficient, but mixing
snprintf and scnprintf is pretty odd, and there's now a discrepancy
between the commit log and the patch which wasn't in my original - I'd
expect a "[use snprintf because xyz]" note added if the change was
intentional.
Rasmus
next prev parent reply other threads:[~2015-11-14 15:24 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-27 22:16 [PATCH] scsi: hpsa: fix multiple issues in path_info_show Rasmus Villemoes
2015-10-28 20:33 ` Don Brace
2015-11-14 15:24 ` Rasmus Villemoes [this message]
2015-11-18 18:27 ` Don Brace
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=87mvuga9jr.fsf@rasmusvillemoes.dk \
--to=linux@rasmusvillemoes.dk \
--cc=JBottomley@odin.com \
--cc=brace77070@gmail.com \
--cc=iss_storagedev@hp.com \
--cc=joseph.t.handzik@hp.com \
--cc=kevin.barnett@pmcs.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=scott.teel@pmcs.com \
--cc=storagedev@pmcs.com \
--cc=thenzl@redhat.com \
/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