[RFC PATCH 1/4] cmd: fuse: Remove custom string functions

[Tom Rini]

On Mon, Mar 17, 2025 at 02:29:39PM +0530, Harsha Vardhan V M wrote:
> 
> 
> On 14/03/25 19:39, Tom Rini wrote:
> > On Fri, Mar 14, 2025 at 07:13:19PM +0530, Harsha Vardhan V M wrote:
> > > 
> > > 
> > > On 13/03/25 21:45, Tom Rini wrote:
> > > > On Thu, Mar 13, 2025 at 05:25:14PM +0530, Harsha Vardhan V M wrote:
> > > > 
> > > > > Remove custom string functions and replace them with normal string
> > > > > functions. Remove the custom strtou32 and replace it with str2long.
> > > > > 
> > > > > Signed-off-by: Harsha Vardhan V M <h-vm at ti.com>
> > > > 
> > > > Thanks for doing this.
> > > > 
> > > > > ---
> > > > >    cmd/fuse.c | 27 ++++++++-------------------
> > > > >    1 file changed, 8 insertions(+), 19 deletions(-)
> > > > > 
> > > > > diff --git a/cmd/fuse.c b/cmd/fuse.c
> > > > > index 598ef496a43..9f489570634 100644
> > > > > --- a/cmd/fuse.c
> > > > > +++ b/cmd/fuse.c
> > > > > @@ -15,17 +15,6 @@
> > > > >    #include <vsprintf.h>
> > > > >    #include <linux/errno.h>
> > > > > -static int strtou32(const char *str, unsigned int base, u32 *result)
> > > > > -{
> > > > > -	char *ep;
> > > > > -
> > > > > -	*result = simple_strtoul(str, &ep, base);
> > > > > -	if (ep == str || *ep != '\0')
> > > > > -		return -EINVAL;
> > > > > -
> > > > > -	return 0;
> > > > > -}
> > > > > -
> > > > >    static int confirm_prog(void)
> > > > >    {
> > > > >    	puts("Warning: Programming fuses is an irreversible operation!\n"
> > > > > @@ -54,14 +43,14 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > >    	argc -= 2 + confirmed;
> > > > >    	argv += 2 + confirmed;
> > > > > -	if (argc < 2 || strtou32(argv[0], 0, &bank) ||
> > > > > -			strtou32(argv[1], 0, &word))
> > > > > +	if (argc < 2 || !(str2long(argv[0], (ulong *)&bank)) ||
> > > > > +			!(str2long(argv[1], (ulong *)&word)))
> > > > 
> > > > I didn't know we had "str2long" which is a differently rarely used
> > > > function. Why not just simple_strtoul inline? Am I missing something?
> > > > Thanks.
> > > 
> > > We cannot use simple_strtoul inline directly because we need proper error
> > > checking to ensure the simple_strtoul conversion was successful. The
> > > str2long function is a wrapper around simple_strtoul and the necessary error
> > > checks. Hence, using str2long here.
> > > Thanks.
> > 
> > I'm sorry I'm still not getting it. We virtually never call str2long. I
> > was taking a quick look at why if anything the 4 callers in the entire
> > tree today have more special error checking requirements than every
> > other command which parses user input.
> > 
> 
> Hi Tom,
> I tried to retain the error checking done by the custom strtou32 function,
> hence replaced the custom strtou32 function with str2long. I did notice that
> in some other files in the cmd/ directory, simple_strtoul is used inline
> with NULL as the endp parameter, like this: simple_strtoul(argv[2], NULL,
> 16).
> 
> Do I proceed with replacing the custom strtou32 function with simple_strtoul
> inline by passing NULL as the endp parameter, similar to the example above?

Yes, that would be good, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250317/f54e4498/attachment.sig>