netdev - Re: [PATCH bpf-next 3/5] tools: libbpf: add a correctly named define for map iteration

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]

Message-ID: <20190227155703.121514a2@cakuba.netronome.com>
Date:   Wed, 27 Feb 2019 15:57:03 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.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, 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..?