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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 2, 2024 at 1:59 PM Eduard Zingerman <eddyz87@xxxxxxxxx> 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@xxxxxxxxx> 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.
>
> [...]





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux