Re: [PATCH v2 2/8] libbpf: Move opts code into dedicated header

[Andrii Nakryiko]

On Mon, Jul 29, 2024 at 10:55 AM Charlie Jenkins <charlie@xxxxxxxxxxxx> wrote:
>
> On Mon, Jul 29, 2024 at 10:01:05AM -0700, Andrii Nakryiko wrote:
> > On Mon, Jul 29, 2024 at 9:46 AM Charlie Jenkins <charlie@xxxxxxxxxxxx> wrote:
> > >
> > > Signed-off-by: Charlie Jenkins <charlie@xxxxxxxxxxxx>
> > > ---
> > >  tools/include/tools/opts.h      | 68 +++++++++++++++++++++++++++++++++++++++++
> > >  tools/lib/bpf/bpf.c             |  1 +
> > >  tools/lib/bpf/btf.c             |  1 +
> > >  tools/lib/bpf/btf_dump.c        |  1 +
> > >  tools/lib/bpf/libbpf.c          |  3 +-
> > >  tools/lib/bpf/libbpf_internal.h | 48 -----------------------------
> > >  tools/lib/bpf/linker.c          |  1 +
> > >  tools/lib/bpf/netlink.c         |  1 +
> > >  tools/lib/bpf/ringbuf.c         |  1 +
> > >  9 files changed, 76 insertions(+), 49 deletions(-)
> > >
> >
> > Nope, sorry, I don't think I want to do this for libbpf. This will
> > just make Github synchronization trickier, and I don't really see a
> > point.
> >
> > I'm totally fine with libperf making a copy of these helpers, though
> > (this is not complicated or tricky code). I also don't think it will
> > change much, so there is little risk of any sort of divergence.
>
> I did this because there were two comments on the previous version of
> this patch that asked to change the functions that were copied over.  I
> had a couple of choices, have the implementations diverge, not change
> the implementation in perf to keep it the same as bpf, update both perf
> and bpf, or share the implementations. I figured the last option was the
> best to avoid immediate divergence. However, both of the comments can be
> safely ignored, and also perhaps divergence doesn't matter.
>

I mean, feel free to diverge. First and foremost the code has to make
sense to specific library and specific use case. If libperf has some
extra things that it needs to enforce or check, by all means. I just
want to avoid unnecessary code sharing, given the code isn't tricky or
complicated, but will complicate libbpf's sync story to Github (libbpf
kind of lives in two places, kernel repo and Github repo).

> - Charlie
>
> >
> > [...]