* [PATCH] scsi: mpi3mr: clean up mpi3mr_print_ioc_info()
@ 2021-09-16 13:26 Dan Carpenter
2021-10-05 3:54 ` Martin K. Petersen
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-09-16 13:26 UTC (permalink / raw)
To: Sathya Prakash Veerichetty
Cc: Kashyap Desai, Sumit Saxena, Sreekanth Reddy,
James E.J. Bottomley, Martin K. Petersen, mpi3mr-linuxdrv.pdl,
linux-scsi, kernel-janitors
This function is more complicated than necessary.
If we change from scnprintf() to snprintf() that let's us remove the
if bytes_wrote < sizeof(protocol) checks. Also we can use
bytes_wrote ? "," : "" to print the comma and remove the separate
if statement and the "is_string_nonempty" variable.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/scsi/mpi3mr/mpi3mr_fw.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index 4a8316c6bd41..0c3dc9288b36 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -3022,7 +3022,6 @@ mpi3mr_print_ioc_info(struct mpi3mr_ioc *mrioc)
char personality[16];
char protocol[50] = {0};
char capabilities[100] = {0};
- bool is_string_nonempty = false;
struct mpi3mr_compimg_ver *fwver = &mrioc->facts.fw_ver;
switch (mrioc->facts.personality) {
@@ -3046,34 +3045,21 @@ mpi3mr_print_ioc_info(struct mpi3mr_ioc *mrioc)
for (i = 0; i < ARRAY_SIZE(mpi3mr_protocols); i++) {
if (mrioc->facts.protocol_flags &
mpi3mr_protocols[i].protocol) {
- if (is_string_nonempty &&
- (bytes_wrote < sizeof(protocol)))
- bytes_wrote += snprintf(protocol + bytes_wrote,
- (sizeof(protocol) - bytes_wrote), ",");
-
- if (bytes_wrote < sizeof(protocol))
- bytes_wrote += snprintf(protocol + bytes_wrote,
- (sizeof(protocol) - bytes_wrote), "%s",
+ bytes_wrote += snprintf(protocol + bytes_wrote,
+ sizeof(protocol) - bytes_wrote, "%s%s",
+ bytes_wrote ? "," : "",
mpi3mr_protocols[i].name);
- is_string_nonempty = true;
}
}
bytes_wrote = 0;
- is_string_nonempty = false;
for (i = 0; i < ARRAY_SIZE(mpi3mr_capabilities); i++) {
if (mrioc->facts.protocol_flags &
mpi3mr_capabilities[i].capability) {
- if (is_string_nonempty &&
- (bytes_wrote < sizeof(capabilities)))
- bytes_wrote += snprintf(capabilities + bytes_wrote,
- (sizeof(capabilities) - bytes_wrote), ",");
-
- if (bytes_wrote < sizeof(capabilities))
- bytes_wrote += snprintf(capabilities + bytes_wrote,
- (sizeof(capabilities) - bytes_wrote), "%s",
+ bytes_wrote += snprintf(capabilities + bytes_wrote,
+ sizeof(capabilities) - bytes_wrote, "%s%s",
+ bytes_wrote ? "," : "",
mpi3mr_capabilities[i].name);
- is_string_nonempty = true;
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: mpi3mr: clean up mpi3mr_print_ioc_info()
2021-09-16 13:26 [PATCH] scsi: mpi3mr: clean up mpi3mr_print_ioc_info() Dan Carpenter
@ 2021-10-05 3:54 ` Martin K. Petersen
2021-10-12 20:35 ` Martin K. Petersen
2021-10-13 8:33 ` Dan Carpenter
2 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2021-10-05 3:54 UTC (permalink / raw)
To: Dan Carpenter
Cc: Sathya Prakash Veerichetty, Kashyap Desai, Sumit Saxena,
Sreekanth Reddy, James E.J. Bottomley, Martin K. Petersen,
mpi3mr-linuxdrv.pdl, linux-scsi, kernel-janitors
Dan,
> This function is more complicated than necessary.
>
> If we change from scnprintf() to snprintf() that let's us remove the
> if bytes_wrote < sizeof(protocol) checks. Also we can use bytes_wrote
> ? "," : "" to print the comma and remove the separate if statement and
> the "is_string_nonempty" variable.
Applied to 5.16/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: mpi3mr: clean up mpi3mr_print_ioc_info()
2021-09-16 13:26 [PATCH] scsi: mpi3mr: clean up mpi3mr_print_ioc_info() Dan Carpenter
2021-10-05 3:54 ` Martin K. Petersen
@ 2021-10-12 20:35 ` Martin K. Petersen
2021-10-13 8:25 ` Dan Carpenter
2021-10-13 8:33 ` Dan Carpenter
2 siblings, 1 reply; 5+ messages in thread
From: Martin K. Petersen @ 2021-10-12 20:35 UTC (permalink / raw)
To: Sathya Prakash Veerichetty, Dan Carpenter
Cc: Martin K . Petersen, kernel-janitors, Kashyap Desai,
Sumit Saxena, mpi3mr-linuxdrv.pdl, James E.J. Bottomley,
linux-scsi, Sreekanth Reddy
On Thu, 16 Sep 2021 16:26:05 +0300, Dan Carpenter wrote:
> This function is more complicated than necessary.
>
> If we change from scnprintf() to snprintf() that let's us remove the
> if bytes_wrote < sizeof(protocol) checks. Also we can use
> bytes_wrote ? "," : "" to print the comma and remove the separate
> if statement and the "is_string_nonempty" variable.
>
> [...]
Applied to 5.16/scsi-queue, thanks!
[1/1] scsi: mpi3mr: clean up mpi3mr_print_ioc_info()
https://git.kernel.org/mkp/scsi/c/76a4f7cc5973
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: mpi3mr: clean up mpi3mr_print_ioc_info()
2021-10-12 20:35 ` Martin K. Petersen
@ 2021-10-13 8:25 ` Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-10-13 8:25 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Sathya Prakash Veerichetty, kernel-janitors, Kashyap Desai,
Sumit Saxena, mpi3mr-linuxdrv.pdl, James E.J. Bottomley,
linux-scsi, Sreekanth Reddy
On Tue, Oct 12, 2021 at 04:35:08PM -0400, Martin K. Petersen wrote:
> On Thu, 16 Sep 2021 16:26:05 +0300, Dan Carpenter wrote:
>
> > This function is more complicated than necessary.
> >
> > If we change from scnprintf() to snprintf() that let's us remove the
> > if bytes_wrote < sizeof(protocol) checks. Also we can use
> > bytes_wrote ? "," : "" to print the comma and remove the separate
> > if statement and the "is_string_nonempty" variable.
> >
> > [...]
>
> Applied to 5.16/scsi-queue, thanks!
>
> [1/1] scsi: mpi3mr: clean up mpi3mr_print_ioc_info()
> https://git.kernel.org/mkp/scsi/c/76a4f7cc5973
Martin, I'm really sorry. I'm looking at this now and I'm so confused
what I was thinking. Why would we change from scnprintf to snprintf?
I clearly intended to do the reverse change and move from snprintf() to
scnprintf(). But then the patch doesn't actually do that. What the
heck???
This has already been applied. I've can't believe I've messed up so
badly twice in a week. :/
The patch doesn't introduce any bugs that weren't there in the original
but it's still horribly wrong. I will send a fix that actually changes
it to scnprintf().
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: mpi3mr: clean up mpi3mr_print_ioc_info()
2021-09-16 13:26 [PATCH] scsi: mpi3mr: clean up mpi3mr_print_ioc_info() Dan Carpenter
2021-10-05 3:54 ` Martin K. Petersen
2021-10-12 20:35 ` Martin K. Petersen
@ 2021-10-13 8:33 ` Dan Carpenter
2 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-10-13 8:33 UTC (permalink / raw)
To: Sathya Prakash Veerichetty
Cc: Kashyap Desai, Sumit Saxena, Sreekanth Reddy,
James E.J. Bottomley, Martin K. Petersen, mpi3mr-linuxdrv.pdl,
linux-scsi, kernel-janitors
On Thu, Sep 16, 2021 at 04:26:05PM +0300, Dan Carpenter wrote:
> @@ -3046,34 +3045,21 @@ mpi3mr_print_ioc_info(struct mpi3mr_ioc *mrioc)
> for (i = 0; i < ARRAY_SIZE(mpi3mr_protocols); i++) {
> if (mrioc->facts.protocol_flags &
> mpi3mr_protocols[i].protocol) {
> - if (is_string_nonempty &&
> - (bytes_wrote < sizeof(protocol)))
> - bytes_wrote += snprintf(protocol + bytes_wrote,
> - (sizeof(protocol) - bytes_wrote), ",");
> -
> - if (bytes_wrote < sizeof(protocol))
> - bytes_wrote += snprintf(protocol + bytes_wrote,
> - (sizeof(protocol) - bytes_wrote), "%s",
Part of the reason I sent this patch is because the
"bytes_wrote < sizeof(protocol)" comparison is off by one. It should
have been "bytes_wrote <= sizeof(protocol)". Being off by one is
harmless here.
I should have written about this in the commit message. I really messed
this one up all sorts of different ways. :/ I'll try do better in the
future.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-13 8:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-16 13:26 [PATCH] scsi: mpi3mr: clean up mpi3mr_print_ioc_info() Dan Carpenter
2021-10-05 3:54 ` Martin K. Petersen
2021-10-12 20:35 ` Martin K. Petersen
2021-10-13 8:25 ` Dan Carpenter
2021-10-13 8:33 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.