[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Z7403OPZUf7WZ5Ai@probook>
Date: Tue, 25 Feb 2025 21:23:40 +0000
From: J. Neuschäfer <j.ne@...teo.net>
To: Alan Stern <stern@...land.harvard.edu>
Cc: j.ne@...teo.net, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-usb@...r.kernel.org, usb-storage@...ts.one-eyed-alien.net,
linux-kernel@...r.kernel.org,
Jonathan Neuschäfer <j.neuschaefer@....net>
Subject: Re: [PATCH] usb: storage: jumpshot: Use const for constant arrays
On Tue, Feb 25, 2025 at 10:11:02AM -0500, Alan Stern wrote:
> On Tue, Feb 25, 2025 at 01:25:03PM +0100, J. Neuschäfer via B4 Relay wrote:
> > From: "J. Neuschäfer" <j.ne@...teo.net>
> >
> > These arrays are not modified. Make them const.
>
> This does not explain _why_ you want to declare these arrays const.
>
> > Signed-off-by: J. Neuschäfer <j.ne@...teo.net>
> > ---
> > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@....net>
>
> There should not be two Signed-off-by lines for the same person. You
> should keep only the one with the email address you used for sending the
> patch. And it should have your full name, just like signing a legal
> document.
Ah sorry, editing error on my part (I started the patch a while ago and
since switched to a different email provider).
>
> > ---
> >
> > This change was compile-tested with ARCH=powerpc
> > ---
> > drivers/usb/storage/jumpshot.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/storage/jumpshot.c b/drivers/usb/storage/jumpshot.c
> > index 39ca84d6859122903de4e64b13e697e8b7d4ab31..089c6f8ac85fbb683cca8b03a2ff06c0ca776167 100644
> > --- a/drivers/usb/storage/jumpshot.c
> > +++ b/drivers/usb/storage/jumpshot.c
> > @@ -367,16 +367,16 @@ static int jumpshot_handle_mode_sense(struct us_data *us,
> > struct scsi_cmnd * srb,
> > int sense_6)
> > {
> > - static unsigned char rw_err_page[12] = {
> > + static const unsigned char rw_err_page[12] = {
> > 0x1, 0xA, 0x21, 1, 0, 0, 0, 0, 1, 0, 0, 0
> > };
> > - static unsigned char cache_page[12] = {
> > + static const unsigned char cache_page[12] = {
> > 0x8, 0xA, 0x1, 0, 0, 0, 0, 0, 0, 0, 0, 0
> > };
> > - static unsigned char rbac_page[12] = {
> > + static const unsigned char rbac_page[12] = {
> > 0x1B, 0xA, 0, 0x81, 0, 0, 0, 0, 0, 0, 0, 0
> > };
> > - static unsigned char timer_page[8] = {
> > + static const unsigned char timer_page[8] = {
> > 0x1C, 0x6, 0, 0, 0, 0
> > };
> > unsigned char pc, page_code;
> > @@ -477,7 +477,7 @@ static int jumpshot_transport(struct scsi_cmnd *srb, struct us_data *us)
> > int rc;
> > unsigned long block, blocks;
> > unsigned char *ptr = us->iobuf;
> > - static unsigned char inquiry_response[8] = {
> > + static const unsigned char inquiry_response[8] = {
> > 0x00, 0x80, 0x00, 0x01, 0x1F, 0x00, 0x00, 0x00
> > };
>
> There's nothing wrong with this. However, several other source files in
> this directory also have static arrays that are never written to. Why
> don't you change all of them?
>
> If you do, it will be best to put the changes for each of the files in
> its own patch, and then submit them as a series of patches.
That's a good idea, I'll do it.
>
> Alan Stern
Powered by blists - more mailing lists