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

[Harsha Vardhan V M <h-vm@ti.com>]

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@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? Please let me know if you have an alternative suggestion.

Thanks,
Harsha