From: Alastair Robertson <ajor@meta.com>
To: <bpf@vger.kernel.org>, <andrii@kernel.org>
Cc: Alastair Robertson <ajor@meta.com>
Subject: Re: [PATCH bpf-next v2 2/2] libbpf: Extend linker API to support in-memory ELF files
Date: Thu, 5 Dec 2024 09:23:18 -0800 [thread overview]
Message-ID: <20241205172318.3481555-1-ajor@meta.com> (raw)
In-Reply-To: <CAEf4BzbZoq1pwq1CZShVzWELC0=eJycFvqPuDXOFEcyu9zYUpA@mail.gmail.com>
> > {
> > - struct src_obj obj =3D {};
> > - int err =3D 0, fd;
> > + int fd, ret;
> >
> > - if (!OPTS_VALID(opts, bpf_linker_file_opts))
> > - return libbpf_err(-EINVAL);
> > + LIBBPF_OPTS(bpf_linker_file_opts, opts);
>
> this is a variable declaration, no empty lines between variable declaration=
> s
I'd originally written it without the extra empty line but got complaints from
checkpatch.pl. Is it ok to just ignore its warnings?
> > +int bpf_linker__add_buf(struct bpf_linker *linker, const char *name,
>
> why is the buffer name passed as an argument instead of through
> opts.filename? let's keep it simple and consistent
>
> and if user didn't care to pass opts.filename, just do some
> "mem:%p+%zu", buf, buf_sz thing
Just because memfd_create() requires a filename so I was treating it as a
required argument for this function too. Happy to change it to this
suggestion though.
All other comments make sense and I'll address them in the next patch.
next prev parent reply other threads:[~2024-12-05 17:23 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-04 16:10 [PATCH bpf-next v2 0/2] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
2024-12-04 16:11 ` [PATCH bpf-next v2 1/2] libbpf: Pull file-opening logic up to top-level functions Alastair Robertson
2024-12-04 16:11 ` [PATCH bpf-next v2 2/2] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
2024-12-04 18:35 ` Andrii Nakryiko
2024-12-05 17:23 ` Alastair Robertson [this message]
2024-12-05 18:24 ` Andrii Nakryiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241205172318.3481555-1-ajor@meta.com \
--to=ajor@meta.com \
--cc=andrii@kernel.org \
--cc=bpf@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox