All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Lothar Waßmann" <LW@KARO-electronics.de>
To: Weijie Gao <weijie.gao@mediatek.com>
Cc: <u-boot@lists.denx.de>,
	GSS_MTK_Uboot_upstream <GSS_MTK_Uboot_upstream@mediatek.com>,
	Tom Rini <trini@konsulko.com>,
	"Lucien . Jheng" <lucienzx159@gmail.com>
Subject: Re: [PATCH v3 2/5] misc: fs_loader: allow using long script name in request_firmware_into_buf_via_script()
Date: Fri, 5 Sep 2025 07:35:28 +0200	[thread overview]
Message-ID: <20250905073528.4cba3123@karo-electronics.de> (raw)
In-Reply-To: <3af4ba436c9d890096d679766cf3d6d6b0b65e71.camel@mediatek.com>

Hi,

On Fri, 5 Sep 2025 09:26:05 +0800 Weijie Gao wrote:
[...]
> > > We'll get compiler warning if using const char *[]:
> > > 
> > >   CC      drivers/misc/fs_loader.o
> > > drivers/misc/fs_loader.c: In function
> > > 'request_firmware_into_buf_via_script':
> > > drivers/misc/fs_loader.c:243:33: warning: passing argument 3 of
> > > 'cmd_process' from incompatible pointer type [-Wincompatible-
> > > pointer-
> > > types]
> > >   243 |         ret = cmd_process(0, 2, args, &repeatable, NULL);
> > >       |                                 ^~~~
> > >       |                                 |
> > >       |                                 const char **
> > >   
> > 
> > OK. But at least:
> >         char *args[2] = { "run", (char *)script_name };
> > should work...  
> 
> You're right. I thought string literals are always const.
> 
> > 
> > 
> > NB: The right fix would be to change the argument type of
> > cmd_process()
> > (and all functions with similar semantics...) to
> > "const char *const argv[]", since cmd_process() need not and should
> > not
> > be able to modify any of the strings within argv[].  
> 
> I agree. But this should be sent in another patch series.
> 
Above all, this should be considered when designing a new API.
There is no reason why a function like cmd_process() should ever be
able to modify the strings it gets passed in the argument list, so they
should always be declared as 'const char *...'.


Lothar Waßmann

  reply	other threads:[~2025-09-05  5:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-03  8:38 [PATCH v3 0/5] Add support for MediaTek MT7987/MT7988 built-in 2.5Gb ethernet PHY (v3) Weijie Gao
2025-09-03  8:38 ` [PATCH v3 1/5] misc: fs_loader: allow returning actual firmware data size in request_firmware_into_buf_via_script() Weijie Gao
2025-09-03  8:38 ` [PATCH v3 2/5] misc: fs_loader: allow using long script name " Weijie Gao
2025-09-03 11:36   ` Lothar Waßmann
2025-09-04  0:23     ` Weijie Gao
2025-09-04  9:28       ` Lothar Waßmann
2025-09-05  1:26         ` Weijie Gao
2025-09-05  5:35           ` Lothar Waßmann [this message]
2025-09-03  8:38 ` [PATCH v3 3/5] net: mediatek: associate PHY device with dts node specified by phy-handle Weijie Gao
2025-09-03  8:38 ` [PATCH v3 4/5] net: phy: Add MediaTek built-in 2.5Gb ethernet PHY driver Weijie Gao
2025-09-03  8:38 ` [PATCH v3 5/5] MAINTAINERS: update ethernet-related file list for MediaTek ARM platform Weijie Gao

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=20250905073528.4cba3123@karo-electronics.de \
    --to=lw@karo-electronics.de \
    --cc=GSS_MTK_Uboot_upstream@mediatek.com \
    --cc=lucienzx159@gmail.com \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    --cc=weijie.gao@mediatek.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 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.