Re: [PATCH] libfs: fix implicitly cast in simple_attr_write_xsigned()

From: Jiasheng Jiang
Date: Sat May 25 2024 - 15:56:18 EST


> On Wed, May 15, 2024 at 03:17:25PM +0000, Jiasheng Jiang wrote:
>> Return 0 to indicate failure and return "len" to indicate success.
>> It was hard to distinguish success or failure if "len" equals the error
>> code after the implicit cast.
>> Moreover, eliminating implicit cast is a better practice.
>
> According to whom?
>

Programmers can easily overlook implicit casts, leading to unknown
behavior (e.g., this bug).
Converting implicit casts to explicit casts can help prevent future
errors.

> Merits of your ex cathedra claims aside, you do realize that functions
> have calling conventions because they are, well, called, right?
> And changing the value returned in such and such case should be
> accompanied with the corresponding change in the _callers_.
>
> Al, wondering if somebody had decided to play with LLM...

As the comment shows that "ret = len; /* on success, claim we got the
whole input */", the return value should be checked to determine whether
it equals "len".

Moreover, if "len" is 0, the previous copy_from_user() will fail and
return an error.
Therefore, 0 is an illegal value for "len". Besides, in the linux kernel,
all the callers of simple_attr_write_xsigned() return the return value of
simple_attr_write_xsigned().

-Jiasheng