On Tue, Nov 15, 2022 at 11:20:34AM IST, Alexei Starovoitov wrote:
> On Tue, Nov 15, 2022 at 12:45:30AM +0530, Kumar Kartikeya Dwivedi wrote:
> > + parse:
> > + if (!tab) {
> > + tab = kzalloc(offsetof(struct btf_struct_metas, types[1]),
> > + GFP_KERNEL | __GFP_NOWARN);
> > + if (!tab)
> > + return ERR_PTR(-ENOMEM);
> > + } else {
> > + struct btf_struct_metas *new_tab;
> > +
> > + new_tab = krealloc(tab, offsetof(struct btf_struct_metas, types[tab->cnt + 1]),
> > + GFP_KERNEL | __GFP_NOWARN);
> > + if (!new_tab) {
> > + ret = -ENOMEM;
> > + goto free;
> > + }
> > + tab = new_tab;
> > + }
>
> If @p is %NULL, krealloc() behaves exactly like kmalloc().
>
> That can help to simplify above a bit?
>
Great suggestion, I made the change.
> > + type = &tab->types[tab->cnt];
> > +
> > + type->btf_id = i;
> > + record = btf_parse_fields(btf, t, BPF_SPIN_LOCK | BPF_LIST_HEAD | BPF_LIST_NODE, t->size);
> > + if (IS_ERR_OR_NULL(record)) {
> > + ret = PTR_ERR_OR_ZERO(record) ?: -EFAULT;
> > + goto free;
> > + }
> > + foffs = btf_parse_field_offs(record);
> > + if (WARN_ON_ONCE(IS_ERR_OR_NULL(foffs))) {
>
> WARN_ON_ONCE ?
> Pls add a comment.
>
I'll drop this, it doesn't have to be a WARN_ON_ONCE. But still we must fail
when field_offs can't be constructed for a valid btf_record. btf_record enables
field discovery, but without field_offs we will never skip over them or rewrite
them to 0 (and zero init is critical for everything else which assumes it).
I'll add a comment that both should be set or we should simply bail.
