On Tue, 3 Dec 2024 at 01:03, Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Thu, Nov 28, 2024 at 4:16 PM Kumar Kartikeya Dwivedi
> <memxor@xxxxxxxxx> wrote:
> > +static struct bpf_reference_state *acquire_reference_state(struct bpf_verifier_env *env, int insn_idx, bool gen_id)
> > {
> > struct bpf_verifier_state *state = env->cur_state;
> > int new_ofs = state->acquired_refs;
> > - int id, err;
> > + int err;
> >
> > err = resize_reference_state(state, state->acquired_refs + 1);
> > if (err)
> > - return err;
> > - id = ++env->id_gen;
> > - state->refs[new_ofs].type = REF_TYPE_PTR;
> > - state->refs[new_ofs].id = id;
> > + return NULL;
> > + if (gen_id)
> > + state->refs[new_ofs].id = ++env->id_gen;
>
> ...
>
> > +static int acquire_reference(struct bpf_verifier_env *env, int insn_idx)
> > +{
> > + struct bpf_reference_state *s;
> > +
> > + s = acquire_reference_state(env, insn_idx, true);
> > + if (!s)
> > + return -ENOMEM;
> > + s->type = REF_TYPE_PTR;
> > + return s->id;
>
> Small nit.
> I think 'bool gen_id' is not very readable, since
> the callsite is not obvious.
> Let's drop the flag and instead do:
> s->id = ++env->id_gen;
> return s->id;
Ok.
>
> > + s = acquire_reference_state(env, insn_idx, false);
> > + s->type = type;
> > + s->id = id;
> > + s->ptr = ptr;
>
> this bit will be easier to read too.
Ok.
