[PATCH RESEND] driver/scsi/mpi3mr.h: Fix build warning for mpi3mr_start_watchdog

[PATCH RESEND] driver/scsi/mpi3mr.h: Fix build warning for mpi3mr_start_watchdog

[Bartlomiej Kubik]

Fix watchdog name truncation.

In function mpi3mr_start_watchdog, watchdog_work_q_name is build
snprintf(mrioc->watchdog_work_q_name,
	sizeof(mrioc->watchdog_work_q_name), "watchdog_%s%d", mrioc->name,
	mrioc->id);

Signed-off-by: Bartlomiej Kubik <kubik.bartlomiej@gmail.com>
---
 drivers/scsi/mpi3mr/mpi3mr.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index 8d4ef49e04d1..5307fcdf216f 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -66,6 +66,7 @@ extern atomic64_t event_counter;

 #define MPI3MR_NAME_LENGTH	64
 #define IOCNAME			"%s: "
+#define MPI3MR_WATCHDOG_NAME_LENGTH	(MPI3MR_NAME_LENGTH + 15)

 #define MPI3MR_DEFAULT_MAX_IO_SIZE	(1 * 1024 * 1024)

@@ -1261,7 +1262,7 @@ struct mpi3mr_ioc {
 	spinlock_t fwevt_lock;
 	struct list_head fwevt_list;

-	char watchdog_work_q_name[50];
+	char watchdog_work_q_name[MPI3MR_WATCHDOG_NAME_LENGTH];
 	struct workqueue_struct *watchdog_work_q;
 	struct delayed_work watchdog_work;
 	spinlock_t watchdog_lock;
--
2.39.5

Re: [PATCH RESEND] driver/scsi/mpi3mr.h: Fix build warning for mpi3mr_start_watchdog

[Martin K. Petersen]

Hi Bartlomiej!

> Fix watchdog name truncation.
>
> In function mpi3mr_start_watchdog, watchdog_work_q_name is build
> snprintf(mrioc->watchdog_work_q_name,
> 	sizeof(mrioc->watchdog_work_q_name), "watchdog_%s%d", mrioc->name,
> 	mrioc->id);

> +#define MPI3MR_WATCHDOG_NAME_LENGTH	(MPI3MR_NAME_LENGTH + 15)

Please document why 15 is the correct number and describe the code path
which leads to the watchdog name being truncated.

Also (and I guess this is a question for Broadcom since I don't have any
mpi3mr hardware so I can't check), as far as I can tell, mrioc->name
already includes mrioc->id so the id will be listed twice in the
watchdog name? Or am I missing something?

-- 
Martin K. Petersen

Re: [PATCH RESEND] driver/scsi/mpi3mr.h: Fix build warning for mpi3mr_start_watchdog

[Bartłomiej Kubik]

Hi Martin,

On Tue, 7 Oct 2025 at 04:25, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
>
> Hi Bartlomiej!
>
> > Fix watchdog name truncation.
> >
> > In function mpi3mr_start_watchdog, watchdog_work_q_name is build
> > snprintf(mrioc->watchdog_work_q_name,
> >       sizeof(mrioc->watchdog_work_q_name), "watchdog_%s%d", mrioc->name,
> >       mrioc->id);
>
> > +#define MPI3MR_WATCHDOG_NAME_LENGTH  (MPI3MR_NAME_LENGTH + 15)
>
> Please document why 15 is the correct number and describe the code path
> which leads to the watchdog name being truncated.
>
> Also (and I guess this is a question for Broadcom since I don't have any
> mpi3mr hardware so I can't check), as far as I can tell, mrioc->name
> already includes mrioc->id so the id will be listed twice in the
> watchdog name? Or am I missing something?
>
> --
> Martin K. Petersen

GCC warn:
drivers/scsi/mpi3mr/mpi3mr_fw.c:2872:60: warning: ‘%s’ directive
output may be truncated writing up to 63 bytes into a region of size
41 [-Wformat-truncation=]

In function mpi3mr_start_watchdog:

The mrioc->watchdog_work_q_name is built from the string "watchdog_" +
mrioc->name[64] and + mrioc->id.
Currently snprintf(...) will truncate this string to size
watchdog_work_q_name[50].

I changed watchdog_work_q_name size to:
-> "watchdog_" are 9 characters.
-> mrioc->name[64] up to 64 characters.
-> mrioc->id is u8 value, 3 characters.
-> plus three extra characters for safety.

Yes, as you suggested, mrioc->id is currently appended twice to the
watchdog_work_q_name, which is unnecessary and makes it harder to
read. I will prepare patch v2 to remove the second occurrence of
mrioc->id from watchdog_work_q_name.

Best regards
Bartłomiej Kubik