Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1272456

RE: [PATCH] scsi: hpsa: fix multiple issues in path_info_show

From Don Brace <Don.Brace@pmcs.com>
Newsgroups linux.kernel
Subject RE: [PATCH] scsi: hpsa: fix multiple issues in path_info_show
Date 2015-11-18 19:30 +0100
Message-ID <qwfEu-6Cj-17@gated-at.bofh.it> (permalink)
References <qokL0-2rO-21@gated-at.bofh.it> <qoFFM-7u4-15@gated-at.bofh.it> <quKW5-5Jb-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Rasmus Villemoes
> Sent: Saturday, November 14, 2015 9:24 AM
> To: Don Brace
> Cc: Joe Handzik; James E.J. Bottomley; Kevin Barnett; Scott Teel; Tomas Henzl;
> iss_storagedev@hp.com; dl Team ESD Storage Dev Support; linux-
> scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] scsi: hpsa: fix multiple issues in path_info_show
> 
> 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

Unintentional.
I'll upload a fix soon.


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

Back to linux.kernel | Previous | Next | Find similar | Unroll thread


Thread

RE: [PATCH] scsi: hpsa: fix multiple issues in path_info_show Don Brace <Don.Brace@pmcs.com> - 2015-11-18 19:30 +0100

csiph-web