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.
>
> Thanks,
> Song
>
> > + break;
> > + case BTF_KIND_ARRAY: {
> > + const struct btf_array *a = btf_array(t);
> > +
> > + err = btf_validate_id(btf, a->type, id);
> > + err = err ?: btf_validate_id(btf, a->index_type, id);
> > + if (err)
> > + return err;
> > + break;
> > + }
> > + case BTF_KIND_STRUCT:
> > + case BTF_KIND_UNION: {
> > + const struct btf_member *m = btf_members(t);
> > +
> > + n = btf_vlen(t);
> > + for (i = 0; i < n; i++, m++) {
> > + err = btf_validate_str(btf, m->name_off, "field name", id);
> > + err = err ?: btf_validate_id(btf, m->type, id);
> > + if (err)
> > + return err;
> > + }
> > + break;
> > + }
> > + case BTF_KIND_ENUM: {
> > + const struct btf_enum *m = btf_enum(t);
> > +
> > + n = btf_vlen(t);
> > + for (i = 0; i < n; i++, m++) {
> > + err = btf_validate_str(btf, m->name_off, "enum name", id);
> > + if (err)
> > + return err;
> > + }
> > + break;
> > + }
> > + case BTF_KIND_ENUM64: {
> > + const struct btf_enum64 *m = btf_enum64(t);
> > +
> > + n = btf_vlen(t);
> > + for (i = 0; i < n; i++, m++) {
> > + err = btf_validate_str(btf, m->name_off, "enum name", id);
> > + if (err)
> > + return err;
> > + }
> > + break;
> > + }
> > + case BTF_KIND_FUNC_PROTO: {
> > + const struct btf_param *m = btf_params(t);
> > +
> > + n = btf_vlen(t);
> > + for (i = 0; i < n; i++, m++) {
> > + err = btf_validate_str(btf, m->name_off, "param name", id);
> > + err = err ?: btf_validate_id(btf, m->type, id);
> > + if (err)
> > + return err;
> > + }
> > + break;
> > + }
> > + case BTF_KIND_DATASEC: {
> > + const struct btf_var_secinfo *m = btf_var_secinfos(t);
> > +
> > + n = btf_vlen(t);
> > + for (i = 0; i < n; i++, m++) {
> > + err = btf_validate_id(btf, m->type, id);
> > + if (err)
> > + return err;
> > + }
> > + break;
> > + }
> > + default:
> > + pr_warn("btf: type [%u]: unrecognized kind %u\n", id, kind);
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> [...]
