lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 27 Feb 2019 16:09:30 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
        bpf@...r.kernel.org, oss-drivers@...ronome.com
Subject: Re: [PATCH bpf-next 3/5] tools: libbpf: add a correctly named define
 for map iteration

On Wed, Feb 27, 2019 at 3:57 PM Jakub Kicinski
<jakub.kicinski@...ronome.com> wrote:
>
> On Wed, 27 Feb 2019 15:47:56 -0800, Andrii Nakryiko wrote:
> > On Wed, Feb 27, 2019 at 3:31 PM Jakub Kicinski
> > <jakub.kicinski@...ronome.com> wrote:
> > >
> > > For historical reasons the helper to loop over maps in an object
> > > is called bpf_map__for_each while it really should be called
> > > bpf_object__for_each_map.  Rename and add a correctly named
> > > define for backward compatibility.
> >
> > Seems like there are at least 3 more functions that are not named correctly:
> > - __bpf_map__iter (__bpf_object__iter_map?)
> > - bpf_map__next (=> bpf_object__next_map?)
> > - bpf_map__prev (=> bpf_object__prev_map?)
> >
> > Let's rename them as well?
>
> At least those are consistently named between programs and maps.
> I'm happy to do the rename if we don't need backward compat, seems
> a little much to add aliases?
>
> > > Switch all in-tree users to the correct name (Quentin).
> > >
> > > Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
> > > Reviewed-by: Quentin Monnet <quentin.monnet@...ronome.com>
>
> > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > > index 6c0168f8bba5..b4652aa1a58a 100644
> > > --- a/tools/lib/bpf/libbpf.h
> > > +++ b/tools/lib/bpf/libbpf.h
> > > @@ -278,10 +278,11 @@ bpf_object__find_map_by_offset(struct bpf_object *obj, size_t offset);
> > >
> > >  LIBBPF_API struct bpf_map *
> > >  bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
> > > -#define bpf_map__for_each(pos, obj)            \
> > > +#define bpf_object__for_each_map(pos, obj)             \
> > >         for ((pos) = bpf_map__next(NULL, (obj));        \
> > >              (pos) != NULL;                             \
> > >              (pos) = bpf_map__next((pos), (obj)))
> > > +#define bpf_map__for_each bpf_object__for_each_map
> >
> > Should we get rid of this as well, instead of accumulating cruft?
>
> Well, we did some gymnastics in the past to maintain backward compat,
> I thought we do need it..?

I'll let others chime in, but, imo, it feels like while we are at
0.0.2 it might be worthwhile to streamline libbpf early on to reduce
confusion later at the expense of early adopters having to do
straightforward conversion right now.

Powered by blists - more mailing lists