Re: [PATCHv2 bpf-next 07/24] libbpf: Add open_elf/close_elf functions

Andrii Nakryiko
2023-06-23T00:33:49Z

On Tue, Jun 20, 2023 at 1:37 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding open_elf/close_elf functions and using it in
> elf_find_func_offset_from_file function. It will be
> used in following changes to save some code.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/lib/bpf/libbpf.c | 62 ++++++++++++++++++++++++++++++------------
> 1 file changed, 44 insertions(+), 18 deletions(-)
>

we should definitely move all this into separate elf.c file

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index cdac368c7ce1..30d9e3b69114 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10927,6 +10927,45 @@ static struct elf_symbol *elf_symbol_iter_next(struct elf_symbol_iter *iter)
> return ret;
> }
>
> +struct elf_fd {
> + Elf *elf;
> + int fd;
> +};
> +
> +static int open_elf(const char *binary_path, struct elf_fd *elf_fd)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + int fd, ret;
> + Elf *elf;
> +
> + if (elf_version(EV_CURRENT) == EV_NONE) {
> + pr_warn("failed to init libelf for %s\n", binary_path);
> + return -LIBBPF_ERRNO__LIBELF;
> + }
> + fd = open(binary_path, O_RDONLY | O_CLOEXEC);
> + if (fd < 0) {
> + ret = -errno;
> + pr_warn("failed to open %s: %s\n", binary_path,
> + libbpf_strerror_r(ret, errmsg, sizeof(errmsg)));

let's add "elf: " prefix for consistency?

> + return ret;
> + }
> + elf = elf_begin(fd, ELF_C_READ_MMAP, NULL);
> + if (!elf) {
> + pr_warn("elf: could not read elf from %s: %s\n", binary_path, elf_errmsg(-1));
> + close(fd);
> + return -LIBBPF_ERRNO__FORMAT;
> + }
> + elf_fd->fd = fd;
> + elf_fd->elf = elf;
> + return 0;
> +}
> +
> +static void close_elf(struct elf_fd *elf_fd)
> +{
> + elf_end(elf_fd->elf);
> + close(elf_fd->fd);
> +}
> +
> /* Find offset of function name in the provided ELF object. "binary_path" is
> * the path to the ELF binary represented by "elf", and only used for error
> * reporting matters. "name" matches symbol name or name@@LIB for library
> @@ -11019,28 +11058,15 @@ static long elf_find_func_offset(Elf *elf, const char *binary_path, const char *
> */
> static long elf_find_func_offset_from_file(const char *binary_path, const char *name)
> {
> - char errmsg[STRERR_BUFSIZE];
> + struct elf_fd elf_fd = {};
> long ret = -ENOENT;
> - Elf *elf;
> - int fd;
>
> - fd = open(binary_path, O_RDONLY | O_CLOEXEC);
> - if (fd < 0) {
> - ret = -errno;
> - pr_warn("failed to open %s: %s\n", binary_path,
> - libbpf_strerror_r(ret, errmsg, sizeof(errmsg)));
> + ret = open_elf(binary_path, &elf_fd);
> + if (ret)
> return ret;
> - }
> - elf = elf_begin(fd, ELF_C_READ_MMAP, NULL);
> - if (!elf) {
> - pr_warn("elf: could not read elf from %s: %s\n", binary_path, elf_errmsg(-1));
> - close(fd);
> - return -LIBBPF_ERRNO__FORMAT;
> - }
>
> - ret = elf_find_func_offset(elf, binary_path, name);
> - elf_end(elf);
> - close(fd);
> + ret = elf_find_func_offset(elf_fd.elf, binary_path, name);
> + close_elf(&elf_fd);
> return ret;
> }
>
> --
> 2.41.0
>