On Fri, Aug 25, 2023 at 11:56 AM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Thu, Aug 24, 2023 at 2:17 PM Song Liu <song@xxxxxxxxxx> wrote:
> >
> > On Thu, Aug 24, 2023 at 1:14 PM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote:
> > >
> > [...]
> > > +
> > > +static int btf_validate_type(const struct btf *btf, const struct btf_type *t, __u32 id)
> > > +{
> > > + __u32 kind = btf_kind(t);
> > > + int err, i, n;
> > > +
> > > + err = btf_validate_str(btf, t->name_off, "type name", id);
> > > + if (err)
> > > + return err;
> > > +
> > > + switch (kind) {
> > > + case BTF_KIND_UNKN:
> > > + case BTF_KIND_INT:
> > > + case BTF_KIND_FWD:
> > > + case BTF_KIND_FLOAT:
> > > + break;
> > > + case BTF_KIND_PTR:
> > > + case BTF_KIND_TYPEDEF:
> > > + case BTF_KIND_VOLATILE:
> > > + case BTF_KIND_CONST:
> > > + case BTF_KIND_RESTRICT:
> > > + case BTF_KIND_FUNC:
> > > + case BTF_KIND_VAR:
> > > + case BTF_KIND_DECL_TAG:
> > > + case BTF_KIND_TYPE_TAG:
> > > + err = btf_validate_id(btf, t->type, id);
> > > + if (err)
> > > + return err;
> >
> > We can "return err;" at the end, and then remove all these "if (err)
> > return err;", right?
>
> in some places, yes, but not where we have loops. And in general I
> find it harder to follow. Keeping each case branch more self-contained
> seems better, personally. I'd prefer to keep it.
We don't really have a loop here. Well, I agree there is benefit to handling
err != 0 immediately, so I don't have a very strong feeling towards the other
way.
Reviewed-by: Song Liu <song@xxxxxxxxxx>
Thanks,
Song
