The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Jijie Shao <shaojijie@huawei.com>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, andrew+netdev@lunn.ch, shenjian15@huawei.com,
	liuyonglong@huawei.com, chenhao418@huawei.com,
	yangshuaisong@h-partners.com, ningwei15@huawei.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 2/3] net: hns3: add missing const qualifier to hclge_log_error() reg parameter
Date: Thu, 6 Aug 2026 12:21:08 +0100	[thread overview]
Message-ID: <20260806112108.GZ51943@horms.kernel.org> (raw)
In-Reply-To: <b632384f-7de3-4411-890f-3b4e148f0de1@huawei.com>

On Thu, Aug 06, 2026 at 03:48:20PM +0800, Jijie Shao wrote:
> 
> on 2026/8/6 0:52, Simon Horman wrote:
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260804130933.880552-2-shaojijie@huawei.com?part=2
> > ---
> > --- Patch [2]: [PATCH net-next 2/3] net: hns3: add missing const qualifier to hclge_log_error() reg parameter ---
> > commit c77e6cbb6d302d584ade8cd89a76f4750fe00694
> > Author: Jijie Shao <shaojijie@huawei.com>
> > 
> > net: hns3: add missing const qualifier to hclge_log_error() reg parameter
> > 
> > This adds a const qualifier to the reg parameter of hclge_log_error() in
> > the hns3 PF error handling code. The parameter is only read inside the
> > function, so the change itself is a straightforward const-correctness
> > cleanup.
> > 
> > > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
> > > index 7e124e2c718da..6093a60d257b1 100644
> > > --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
> > > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
> > > @@ -1762,7 +1762,7 @@ static const struct hclge_hw_type_id hclge_hw_type_id_st[] = {
> > >   	},
> > >   };
> > > -static void hclge_log_error(struct device *dev, char *reg,
> > > +static void hclge_log_error(struct device *dev, const char *reg,
> > >   			    const struct hclge_hw_error *err,
> > >   			    u32 err_sts, unsigned long *reset_requests)
> > >   {
> > [Severity: Low]
> > [Finding: main-1-0]
> > [Sources: opus-5]
> > This isn't a bug in the code, but is the justification in the changelog
> > accurate?  The commit message says:
> > 
> >      Callers pass const strings, causing a compiler warning about
> >      discarding the 'const' qualifier.
> > 
> > hclge_log_error() is static, and every call site is in hclge_err.c.  They
> > all pass a bare string literal, for example in hclge_handle_mpf_ras_error():
> > 
> >      hclge_log_error(dev, "IMP_TCM_ECC_INT_STS",
> >                      &hclge_imp_tcm_ecc_int[0], status,
> >                      &ae_dev->hw_err_reset_req);
> > 
> > and similarly for "SSU_ECC_MULTI_BIT_INT_0" and the remaining register
> > names down to "PPU_PF_ABNORMAL_INT_ST" in the MSI-X handlers.
> > 
> > In C a string literal has type char[N] rather than const char[N], so
> > passing one to a char *reg parameter does not discard a qualifier, and the
> > kernel build does not enable -Wwrite-strings.  None of the call sites pass
> > a value held in a const char * lvalue (such as
> > struct hclge_mod_reg_info::reg_name), which is the case that would produce
> > the described diagnostic.
> > 
> > Could the changelog be reworded to describe this as a const-correctness
> > cleanup instead of a build warning fix?  As written it suggests a broken
> > build and reads like a stable backport candidate.
> 
> The warning is real in our internal build, which enables
> -Wwrite-strings. Would it be acceptable to keep the commit message
> as-is, or should I reword it to const-correctness cleanup and note
> the internal build context in the changelog?
> 
> This series is cleanup, not bugfix — no Fixes: tags, not intended for stable.

Thanks, I agree with your analysis.
Sorry for the noise on this one.

Reviewed-by: Simon Horman <horms@kernel.org>


  reply	other threads:[~2026-08-06 11:21 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04 13:09 [PATCH net-next 0/3] net: hns3: some cleanups for hns3 driver Jijie Shao
2026-08-04 13:09 ` [PATCH net-next 1/3] net: hns3: set msg->desc to NULL after kfree in hclge_query_reg_info() Jijie Shao
2026-08-05 16:52   ` Simon Horman
2026-08-06  7:32     ` Jijie Shao
2026-08-06 11:26       ` Simon Horman
2026-08-04 13:09 ` [PATCH net-next 2/3] net: hns3: add missing const qualifier to hclge_log_error() reg parameter Jijie Shao
2026-08-05 16:52   ` Simon Horman
2026-08-06  7:48     ` Jijie Shao
2026-08-06 11:21       ` Simon Horman [this message]
2026-08-04 13:09 ` [PATCH net-next 3/3] net: hns3: use txqueue parameter directly in ndo_tx_timeout Jijie Shao
2026-08-05 16:52   ` Simon Horman
2026-08-06 11:39     ` Jijie Shao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260806112108.GZ51943@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=chenhao418@huawei.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liuyonglong@huawei.com \
    --cc=netdev@vger.kernel.org \
    --cc=ningwei15@huawei.com \
    --cc=pabeni@redhat.com \
    --cc=shaojijie@huawei.com \
    --cc=shenjian15@huawei.com \
    --cc=yangshuaisong@h-partners.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox