[PATCH] smb: client: Move an error code assignment in smb3_init_transform_rq()

[Steve French]

Also when a patch doesn't shrink code, but even grows it by one line, and
doesn't make it much clearer, it is probably not worth changing (it
complicates future backports of real fixes in the future eg). Let's limit
these to those that shrink code and make code clearer (or ideally fix
potential bugs)

Thanks,

Steve

On Wed, Oct 8, 2025, 12:19 PM Henrique Carvalho <henrique.carvalho at suse.com>
wrote:

> Hi Markus,
>
> On 10/8/25 2:04 PM, Markus Elfring wrote:
> > From: Markus Elfring <elfring at users.sourceforge.net>
> > Date: Wed, 8 Oct 2025 18:48:28 +0200
> >
> > Convert an initialisation for the variable “rc” into an error code
> > assignment at the end of this function implementation.
> >
> > Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
> > ---
> >  fs/smb/client/smb2ops.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> > index 7c3e96260fd4..2513270ac596 100644
> > --- a/fs/smb/client/smb2ops.c
> > +++ b/fs/smb/client/smb2ops.c
> > @@ -4596,7 +4596,7 @@ smb3_init_transform_rq(struct TCP_Server_Info
> *server, int num_rqst,
> >  {
> >       struct smb2_transform_hdr *tr_hdr = new_rq[0].rq_iov[0].iov_base;
> >       unsigned int orig_len = 0;
> > -     int rc = -ENOMEM;
> > +     int rc;
> >
> >       for (int i = 1; i < num_rqst; i++) {
> >               struct smb_rqst *old = &old_rq[i - 1];
> > @@ -4611,7 +4611,7 @@ smb3_init_transform_rq(struct TCP_Server_Info
> *server, int num_rqst,
> >               if (size > 0) {
> >                       buffer = cifs_alloc_folioq_buffer(size);
> >                       if (!buffer)
> > -                             goto err_free;
> > +                             goto e_nomem;
> >
> >                       new->rq_buffer = buffer;
> >                       iov_iter_folio_queue(&new->rq_iter, ITER_SOURCE,
> > @@ -4634,6 +4634,8 @@ smb3_init_transform_rq(struct TCP_Server_Info
> *server, int num_rqst,
> >
> >       return rc;
> >
> > +e_nomem:
> > +     rc = -ENOMEM;
> >  err_free:
> >       smb3_free_compound_rqst(num_rqst - 1, &new_rq[1]);
> >       return rc;
>
> I don't think this change improves readability.
>
> I understand that making the assignment explicit is good, but why not
> simply set rc to -ENOMEM if !buffer and then goto err_free?
>
> Also, I think its a bit confusing having inconsistent naming styles `e_`
> `err_`...
>
> --
> Henrique
> SUSE Labs
>
>