[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: [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