| Message ID | 20240326223825.4084412-4-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 954fd908f177604d4cce77e2a88cc50b29bad5ff |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | enabled -Wformat-truncation for clang | expand |
Hi, >-----Original Message----- >From: Arnd Bergmann <arnd@kernel.org> >Sent: Wednesday, March 27, 2024 4:08 AM >To: llvm@lists.linux.dev; Ariel Elior <aelior@marvell.com>; Manish Chopra ><manishc@marvell.com> >Cc: Arnd Bergmann <arnd@arndb.de>; David S. Miller <davem@davemloft.net>; >Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo >Abeni <pabeni@redhat.com>; Nathan Chancellor <nathan@kernel.org>; Nick >Desaulniers <ndesaulniers@google.com>; Bill Wendling <morbo@google.com>; >Justin Stitt <justinstitt@google.com>; Simon Horman <horms@kernel.org>; >Konstantin Khorenko <khorenko@virtuozzo.com>; Sudarsana Reddy Kalluru ><sudarsana.kalluru@cavium.com>; netdev@vger.kernel.org; linux- >kernel@vger.kernel.org >Subject: [PATCH 3/9] qed: avoid truncating work queue length > >From: Arnd Bergmann <arnd@arndb.de> > >clang complains that the temporary string for the name passed into >alloc_workqueue() is too short for its contents: > >drivers/net/ethernet/qlogic/qed/qed_main.c:1218:3: error: 'snprintf' will always >be truncated; specified size is 16, but format string expands to at least 18 [- >Werror,-Wformat-truncation] > >There is no need for a temporary buffer, and the actual name of a workqueue >is 32 bytes (WQ_NAME_LEN), so just use the interface as intended to avoid >the truncation. > >Fixes: 59ccf86fe69a ("qed: Add driver infrastucture for handling mfw requests.") >Signed-off-by: Arnd Bergmann <arnd@arndb.de> >--- > drivers/net/ethernet/qlogic/qed/qed_main.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > >diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c >b/drivers/net/ethernet/qlogic/qed/qed_main.c >index c278f8893042..8159b4c315b5 100644 >--- a/drivers/net/ethernet/qlogic/qed/qed_main.c >+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c >@@ -1206,7 +1206,6 @@ static void qed_slowpath_task(struct work_struct >*work) > static int qed_slowpath_wq_start(struct qed_dev *cdev) > { > struct qed_hwfn *hwfn; >- char name[NAME_SIZE]; > int i; > > if (IS_VF(cdev)) >@@ -1215,11 +1214,11 @@ static int qed_slowpath_wq_start(struct qed_dev >*cdev) > for_each_hwfn(cdev, i) { > hwfn = &cdev->hwfns[i]; > >- snprintf(name, NAME_SIZE, "slowpath-%02x:%02x.%02x", >- cdev->pdev->bus->number, >- PCI_SLOT(cdev->pdev->devfn), hwfn->abs_pf_id); >+ hwfn->slowpath_wq = alloc_workqueue("slowpath- >%02x:%02x.%02x", >+ 0, 0, cdev->pdev->bus->number, >+ PCI_SLOT(cdev->pdev->devfn), >+ hwfn->abs_pf_id); Confused. This should be alloc_workqueue("slowpath-%02x:%02x.%02x", cdev->pdev->bus->number, PCI_SLOT(cdev->pdev->devfn), hwfn->abs_pf_id, 0, 0); Right? Thanks, Sundeep > >- hwfn->slowpath_wq = alloc_workqueue(name, 0, 0); > if (!hwfn->slowpath_wq) { > DP_NOTICE(hwfn, "Cannot create slowpath >workqueue\n"); > return -ENOMEM; >-- >2.39.2 >
On Wed, Mar 27, 2024, at 15:04, Subbaraya Sundeep Bhatta wrote: >>- snprintf(name, NAME_SIZE, "slowpath-%02x:%02x.%02x", >>- cdev->pdev->bus->number, >>- PCI_SLOT(cdev->pdev->devfn), hwfn->abs_pf_id); >>+ hwfn->slowpath_wq = alloc_workqueue("slowpath- >>%02x:%02x.%02x", >>+ 0, 0, cdev->pdev->bus->number, >>+ PCI_SLOT(cdev->pdev->devfn), >>+ hwfn->abs_pf_id); > > Confused. This should be alloc_workqueue("slowpath-%02x:%02x.%02x", > cdev->pdev->bus->number, PCI_SLOT(cdev->pdev->devfn), hwfn->abs_pf_id, > 0, 0); > Right? I still think my version is the right one here, see the prototype: __printf(1, 4) struct workqueue_struct * alloc_workqueue(const char *fmt, unsigned int flags, int max_active, ...); so the first argument in the format, while the printf arguments start after the flags and max_active arguments that are still both set to zero. Arnd
Hi, >-----Original Message----- >From: Arnd Bergmann <arnd@arndb.de> >Sent: Wednesday, March 27, 2024 9:05 PM >To: Subbaraya Sundeep Bhatta <sbhatta@marvell.com>; Arnd Bergmann ><arnd@kernel.org>; llvm@lists.linux.dev; Ariel Elior <aelior@marvell.com>; >Manish Chopra <manishc@marvell.com> >Cc: David S . Miller <davem@davemloft.net>; Eric Dumazet ><edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni ><pabeni@redhat.com>; Nathan Chancellor <nathan@kernel.org>; Nick >Desaulniers <ndesaulniers@google.com>; Bill Wendling <morbo@google.com>; >Justin Stitt <justinstitt@google.com>; Simon Horman <horms@kernel.org>; >Konstantin Khorenko <khorenko@virtuozzo.com>; Sudarsana Reddy Kalluru ><sudarsana.kalluru@cavium.com>; Netdev <netdev@vger.kernel.org>; linux- >kernel@vger.kernel.org >Subject: Re: [EXTERNAL] [PATCH 3/9] qed: avoid truncating work queue length > >On Wed, Mar 27, 2024, at 15:04, Subbaraya Sundeep Bhatta wrote: > >>>- snprintf(name, NAME_SIZE, "slowpath-%02x:%02x.%02x", >>>- cdev->pdev->bus->number, >>>- PCI_SLOT(cdev->pdev->devfn), hwfn->abs_pf_id); >>>+ hwfn->slowpath_wq = alloc_workqueue("slowpath- >>>%02x:%02x.%02x", >>>+ 0, 0, cdev->pdev->bus->number, >>>+ PCI_SLOT(cdev->pdev->devfn), >>>+ hwfn->abs_pf_id); >> >> Confused. This should be alloc_workqueue("slowpath-%02x:%02x.%02x", >> cdev->pdev->bus->number, PCI_SLOT(cdev->pdev->devfn), hwfn->abs_pf_id, >> 0, 0); >> Right? > >I still think my version is the right one here, see the >prototype: > >__printf(1, 4) struct workqueue_struct * >alloc_workqueue(const char *fmt, unsigned int flags, int max_active, ...); > >so the first argument in the format, while the printf arguments >start after the flags and max_active arguments that are still both >set to zero. > My bad. Got it Thanks, Sundeep > Arnd
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c index c278f8893042..8159b4c315b5 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_main.c +++ b/drivers/net/ethernet/qlogic/qed/qed_main.c @@ -1206,7 +1206,6 @@ static void qed_slowpath_task(struct work_struct *work) static int qed_slowpath_wq_start(struct qed_dev *cdev) { struct qed_hwfn *hwfn; - char name[NAME_SIZE]; int i; if (IS_VF(cdev)) @@ -1215,11 +1214,11 @@ static int qed_slowpath_wq_start(struct qed_dev *cdev) for_each_hwfn(cdev, i) { hwfn = &cdev->hwfns[i]; - snprintf(name, NAME_SIZE, "slowpath-%02x:%02x.%02x", - cdev->pdev->bus->number, - PCI_SLOT(cdev->pdev->devfn), hwfn->abs_pf_id); + hwfn->slowpath_wq = alloc_workqueue("slowpath-%02x:%02x.%02x", + 0, 0, cdev->pdev->bus->number, + PCI_SLOT(cdev->pdev->devfn), + hwfn->abs_pf_id); - hwfn->slowpath_wq = alloc_workqueue(name, 0, 0); if (!hwfn->slowpath_wq) { DP_NOTICE(hwfn, "Cannot create slowpath workqueue\n"); return -ENOMEM;