Re: [RFC bpf-next v1 4/8] selftests/bpf: extract utility function for BPF disassembly

Andrii Nakryiko
2024-07-02T21:16:34Z

On Tue, Jul 2, 2024 at 1:59 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Mon, 2024-07-01 at 17:41 -0700, Andrii Nakryiko wrote:
> > On Sat, Jun 29, 2024 at 2:48 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > >
> > > uint32_t disasm_insn(struct bpf_insn *insn, char *buf, size_t buf_sz);
> >
> > or you can return `struct bpf_insn *` which will point to the next
> > hypothetical instruction?
>
> Not sure if it simplifies clients, e.g. from this patch, the following:
>
> + for (i = skip_first_insn ? 1 : 0; i < cnt;) {
> + i += disasm_insn(buf + i, insn_buf, sizeof(insn_buf));
> + fprintf(prog_out, "%s\n", insn_buf);
> + }
>
> Would become:
>
> + for (i = buf + skip_first_insn ? 1 : 0; i < buf + cnt;) {
> + i = disasm_insn(buf + i, insn_buf, sizeof(insn_buf));
> + fprintf(prog_out, "%s\n", insn_buf);
> + }
>

struct bpf_insn *insn = skip_first_insn ? buf + 1 : buf, *insn_end = buf + cnt;

while (insn != insn_end) {
insn = disasm_insn(insn, insn_buf, sizeof(insn_buf));
fprintf(prog_out, "%s\n", insn_buf);
}

less addition, but it's simple enough in both cases, of course (I just
find 1 or 2 as a result kind of a bad contract, but whatever)

> idk, can change if you insist.
>
> [...]
>
> > > + sscanf(buf, "(%*[^)]) %n", &pfx_end);
> >
> > let me simplify this a bit ;)
> >
> > pfx_end = 5;
> >
> > not as sophisticated, but equivalent
>
> Okay :(

if 5 makes you sad, do keep sscanf(), of course, no worries :)

>
> >
> > > + sscanf(buf, "(%*[^)]) call %*[^#]%n", &sfx_start);
> >
> > is it documented that sfx_start won't be updated if sscanf() doesn't
> > successfully match?
> >
> > if not, maybe let's do something like below
> >
> > if (strcmp(buf + 5, "call ", 5) == 0 && (tmp = strrchr(buf, '#')))
> > sfx_start = tmp - buf;
>
> Will change, the doc is obscure.
>
> …