Received-SPF: pass (google.com: domain of linux-kernel+bounces-22264-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <6cbcf640-55e5-2f11-4a09-716fe681c0d2@web.de> <87b65f8e-abde-2aff-4da8-df6e0b464677@web.de>
 <05d334af-1a0f-4498-b57d-36a783288f07@web.de> <CAOQ4uxiRaTQyT1nxeRD7B89=VuA+KKEqi01LL1kqfJ17-qKKpw@mail.gmail.com>
 <d912872a-e70a-4e5d-aabe-26f289507f44@web.de> <CAOQ4uxjsuYy8BzgaHXaNa-S0+HZ_P-Fb1UEgOkbrdxQPz0Y6rQ@mail.gmail.com>
 <1df4916d-421c-4c87-8503-5a36934d03d8@web.de>
In-Reply-To: <1df4916d-421c-4c87-8503-5a36934d03d8@web.de>
From: Amir Goldstein <amir73il@gmail.com>
Date: Wed, 10 Jan 2024 15:45:37 +0200
Message-ID: <CAOQ4uxgx8m5Q6+DyfVaZUyFhFu_TAyOWLdHHUJfgOOkTDYMuiA@mail.gmail.com>
Subject: Re: [0/4] overlayfs: Adjustments for ovl_fill_super()
To: Markus Elfring <Markus.Elfring@web.de>
Cc: linux-unionfs@vger.kernel.org, kernel-janitors@vger.kernel.org, 
	Miklos Szeredi <miklos@szeredi.hu>, cocci@inria.fr, LKML <linux-kernel@vger.kernel.org>, 
	Christian Brauner <brauner@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Jan 10, 2024 at 3:33=E2=80=AFPM Markus Elfring <Markus.Elfring@web.=
de> wrote:
>
> >>>> See also:
> >>>> https://lore.kernel.org/cocci/87b65f8e-abde-2aff-4da8-df6e0b464677@w=
eb.de/
> >>>> https://sympa.inria.fr/sympa/arc/cocci/2023-03/msg00115.html
> >>>
> >>> I will queue cleanup patches 1-2,
> >>
> >> Thanks for this positive feedback.
> >
> > Sorry, these patches do not apply to master branch and patch 1
> > is no longer correct in master branch and the new mount api changes.
>
> Do you want that I adapt the linked development ideas to the current situ=
ation
> a bit more?
>

No thanks.
Patch 1 just doesn't work for the new mount api.

>
> >>>                                   but I do not like patches 3/4 and 4=
/4.
> >>> I do not think that they make the code better to read or maintain.
> >>
> >> I would appreciate if the details for such change reluctance can be cl=
arified better.
> >
> > patch 3:
> > I much rather a single error handling label that takes care of
> > all the cleanups - it is harder to make mistakes and jump to
> > the wrong label when adding new error conditions.
>
> There are different coding style preferences involved.
>
> See also:
> https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+g=
oto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resourc=
es
>

As long as coding styles are not mandatory
I prefer what we have right now.

>
> > patch 4:
> > Overlayfs uses this coding style all over the place
> >
> >   err =3D -ENOMEM;
> >   ofs->creator_cred =3D cred =3D prepare_creds();
> >   if (!cred)
> >       goto out_free_ofs;
> >
> > I don't see the benefit in making err =3D -ENOMEM conditional.
> > I don't see the style after your patch as clearly better than before.
>
> Can it be nicer to set error codes only in exceptional data processing si=
tuations?
>

It's a matter of taste.
I'll stay with what we have.

Thanks,
Amir.