[PATCH] smb: client: Simplify a return statement in get_smb2_acl_by_path()

[PATCH] smb: client: Simplify a return statement in get_smb2_acl_by_path()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Oct 2025 21:56:34 +0200

Return an error pointer without referencing another local variable
in an if branch of this function implementation.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/smb/client/smb2ops.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 7c3e96260fd4..bb5eda032aa4 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -3216,9 +3216,8 @@ get_smb2_acl_by_path(struct cifs_sb_info *cifs_sb,
 
 	utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
 	if (!utf16_path) {
-		rc = -ENOMEM;
 		free_xid(xid);
-		return ERR_PTR(rc);
+		return ERR_PTR(-ENOMEM);
 	}
 
 	oparms = (struct cifs_open_parms) {
-- 
2.51.0

Re: [PATCH] smb: client: Simplify a return statement in get_smb2_acl_by_path()

[Steve French]

merged into cifs-2.6.git for-next

On Wed, Oct 8, 2025 at 3:02 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 8 Oct 2025 21:56:34 +0200
>
> Return an error pointer without referencing another local variable
> in an if branch of this function implementation.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  fs/smb/client/smb2ops.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 7c3e96260fd4..bb5eda032aa4 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -3216,9 +3216,8 @@ get_smb2_acl_by_path(struct cifs_sb_info *cifs_sb,
>
>         utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
>         if (!utf16_path) {
> -               rc = -ENOMEM;
>                 free_xid(xid);
> -               return ERR_PTR(rc);
> +               return ERR_PTR(-ENOMEM);
>         }
>
>         oparms = (struct cifs_open_parms) {
> --
> 2.51.0
>
>


-- 
Thanks,

Steve

Re: smb: client: Simplify a return statement in get_smb2_acl_by_path()

[Markus Elfring]

> This is an example of one that is probably slightly worth it,

Thanks for such a positive indication.


> it shrinks one line of code, and also doesn't have risk,

Similar source code refinements might become also interesting and helpful.


> but at least three of the others today don't shrink and sometimes grow lines of code (and don't fix anything )

Further update candidates can be found and eventually transformed also with the help
of the semantic patch language (Coccinelle software).


> so are unlikely to be worth it since they slightly increase risk of adding difficulty to stable backports of future fixes

I hope that such change resistance can be reconsidered.

Regards,
Markus

Re: smb: client: Simplify a return statement in get_smb2_acl_by_path()

[Markus Elfring]

> Three of the four you just sent today, grow the code slightly,

How much will this observation matter?


> and don't fix anything,

Do these change suggestions indicate possible improvements according to specific design aspects?


> the remaining one could be considered

I am curious how the patch reviews will be evolving further.

Regards,
Markus

Re: smb: client: Simplify a return statement in get_smb2_acl_by_path()

[Steve French]

On Thu, Oct 9, 2025 at 9:00 AM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> > Three of the four you just sent today, grow the code slightly,
>
> How much will this observation matter?

Remember the goals here, the priorities:
1) Fixes for bugs
2) Performance improvements
3) Features that add support for missing Linux requirements (e.g.
adding O_TMPFILE or "rename_exchange support)
and then lower priority but still somewhat useful:
4) cleanup patches that remove unneeded code (although has to be
balanced against how it can hurt backports of important fixes),
ie cleanup that makes code *smaller* and therefore slightly easier to
read and maintain
5) cleanup code that adds or updates comments clarifying confusing code

Obviously "cleanup" patches that don't shrink code are often useless
(as they potentially break backports of patches for no value).




-- 
Thanks,

Steve

Re: [PATCH] smb: client: Simplify a return statement in get_smb2_acl_by_path()

[Steve French]

As pointed out by the kernel test robot a few minutes ago, this patch
would introduce a regression (uninitialized rc variable in free_xid
macro), so will remove this patch from for-next.


On Wed, Oct 8, 2025 at 3:02 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 8 Oct 2025 21:56:34 +0200
>
> Return an error pointer without referencing another local variable
> in an if branch of this function implementation.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  fs/smb/client/smb2ops.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 7c3e96260fd4..bb5eda032aa4 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -3216,9 +3216,8 @@ get_smb2_acl_by_path(struct cifs_sb_info *cifs_sb,
>
>         utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
>         if (!utf16_path) {
> -               rc = -ENOMEM;
>                 free_xid(xid);
> -               return ERR_PTR(rc);
> +               return ERR_PTR(-ENOMEM);
>         }
>
>         oparms = (struct cifs_open_parms) {
> --
> 2.51.0
>
>


-- 
Thanks,

Steve

Re: [PATCH] smb: client: Simplify a return statement in get_smb2_acl_by_path()

[Markus Elfring]

> As pointed out by the kernel test robot a few minutes ago, this patch
> would introduce a regression (uninitialized rc variable in free_xid
> macro), so will remove this patch from for-next.

Will this (clang) compiler report be reconsidered once more under other circumstances?


…>> +++ b/fs/smb/client/smb2ops.c
>> @@ -3216,9 +3216,8 @@ get_smb2_acl_by_path(struct cifs_sb_info *cifs_sb,
>>
>>         utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
>>         if (!utf16_path) {
>> -               rc = -ENOMEM;
>>                 free_xid(xid);
>> -               return ERR_PTR(rc);
>> +               return ERR_PTR(-ENOMEM);
>>         }
>>
>>         oparms = (struct cifs_open_parms) {
…

Can it be that the uninitialized rc variable would not really matter for
the adjusted statements in such an if branch?

Regards,
Markus

Re: [PATCH] smb: client: Simplify a return statement in get_smb2_acl_by_path()

[Markus Elfring]

> As pointed out by the kernel test robot a few minutes ago, this patch
> would introduce a regression (uninitialized rc variable in free_xid
> macro), so will remove this patch from for-next.

Is there a need to express the tracing of return values in any other ways?
https://elixir.bootlin.com/linux/v6.17.1/source/fs/smb/client/cifsproto.h#L49-L58

Regards,
Markus