* [PATCH bpf-next v2 4/5] bpftool: clean-up usage of libbpf_get_error()
@ 2022-11-09 7:45 Sahid Orentino Ferdjaoui
2022-11-10 0:03 ` Andrii Nakryiko
0 siblings, 1 reply; 2+ messages in thread
From: Sahid Orentino Ferdjaoui @ 2022-11-09 7:45 UTC (permalink / raw)
To: bpf, ast, daniel, andrii, quentin
Cc: martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo,
jolsa, Sahid Orentino Ferdjaoui
bpftool is now totally compliant with libbpf 1.0 mode and is not
expected to be compiled with pre-1.0, let's clean-up the usage of
libbpf_get_error().
In some places functions that are returning result of
libbpf_get_error() will now return errno. This won't impact the
functionality as the caller checks for 0/non-0 for success/failure.
sidenode: We can remove the checks on NULL pointers before calling
btf__free() because that function already does the check.
Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@industrialdiscipline.com>
---
tools/bpf/bpftool/btf.c | 17 +++++++----------
tools/bpf/bpftool/btf_dumper.c | 2 +-
tools/bpf/bpftool/gen.c | 11 ++++-------
tools/bpf/bpftool/iter.c | 6 ++----
tools/bpf/bpftool/main.c | 7 +++----
tools/bpf/bpftool/map.c | 15 +++++++--------
tools/bpf/bpftool/prog.c | 10 +++++-----
tools/bpf/bpftool/struct_ops.c | 15 +++++++--------
8 files changed, 36 insertions(+), 47 deletions(-)
diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index 68a70ac03c80..ed39b24e39d2 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -467,9 +467,8 @@ static int dump_btf_c(const struct btf *btf,
int err = 0, i;
d = btf_dump__new(btf, btf_dump_printf, NULL, NULL);
- err = libbpf_get_error(d);
- if (err)
- return err;
+ if (!d)
+ return errno;
printf("#ifndef __VMLINUX_H__\n");
printf("#define __VMLINUX_H__\n");
@@ -512,11 +511,9 @@ static struct btf *get_vmlinux_btf_from_sysfs(void)
struct btf *base;
base = btf__parse(sysfs_vmlinux, NULL);
- if (libbpf_get_error(base)) {
- p_err("failed to parse vmlinux BTF at '%s': %ld\n",
- sysfs_vmlinux, libbpf_get_error(base));
- base = NULL;
- }
+ if (!base)
+ p_err("failed to parse vmlinux BTF at '%s': %d\n",
+ sysfs_vmlinux, errno);
return base;
}
@@ -634,8 +631,8 @@ static int do_dump(int argc, char **argv)
base = get_vmlinux_btf_from_sysfs();
btf = btf__parse_split(*argv, base ?: base_btf);
- err = libbpf_get_error(btf);
if (!btf) {
+ err = errno;
p_err("failed to load BTF from %s: %s",
*argv, strerror(errno));
goto done;
@@ -681,8 +678,8 @@ static int do_dump(int argc, char **argv)
}
btf = btf__load_from_kernel_by_id_split(btf_id, base_btf);
- err = libbpf_get_error(btf);
if (!btf) {
+ err = errno;
p_err("get btf by id (%u): %s", btf_id, strerror(errno));
goto done;
}
diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
index 19924b6ce796..eda71fdfe95a 100644
--- a/tools/bpf/bpftool/btf_dumper.c
+++ b/tools/bpf/bpftool/btf_dumper.c
@@ -75,7 +75,7 @@ static int dump_prog_id_as_func_ptr(const struct btf_dumper *d,
goto print;
prog_btf = btf__load_from_kernel_by_id(info.btf_id);
- if (libbpf_get_error(prog_btf))
+ if (!prog_btf)
goto print;
func_type = btf__type_by_id(prog_btf, finfo.type_id);
if (!func_type || !btf_is_func(func_type))
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index cf8b4e525c88..d00b4e1b99ba 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -252,9 +252,8 @@ static int codegen_datasecs(struct bpf_object *obj, const char *obj_name)
int err = 0;
d = btf_dump__new(btf, codegen_btf_dump_printf, NULL, NULL);
- err = libbpf_get_error(d);
- if (err)
- return err;
+ if (!d)
+ return errno;
bpf_object__for_each_map(map, obj) {
/* only generate definitions for memory-mapped internal maps */
@@ -976,13 +975,11 @@ static int do_skeleton(int argc, char **argv)
/* log_level1 + log_level2 + stats, but not stable UAPI */
opts.kernel_log_level = 1 + 2 + 4;
obj = bpf_object__open_mem(obj_data, file_sz, &opts);
- err = libbpf_get_error(obj);
- if (err) {
+ if (!obj) {
char err_buf[256];
- libbpf_strerror(err, err_buf, sizeof(err_buf));
+ libbpf_strerror(errno, err_buf, sizeof(err_buf));
p_err("failed to open BPF object file: %s", err_buf);
- obj = NULL;
goto out;
}
diff --git a/tools/bpf/bpftool/iter.c b/tools/bpf/bpftool/iter.c
index a3e6b167153d..ab6f1b2befe7 100644
--- a/tools/bpf/bpftool/iter.c
+++ b/tools/bpf/bpftool/iter.c
@@ -48,8 +48,7 @@ static int do_pin(int argc, char **argv)
}
obj = bpf_object__open(objfile);
- err = libbpf_get_error(obj);
- if (err) {
+ if (!obj) {
p_err("can't open objfile %s", objfile);
goto close_map_fd;
}
@@ -67,8 +66,7 @@ static int do_pin(int argc, char **argv)
}
link = bpf_program__attach_iter(prog, &iter_opts);
- err = libbpf_get_error(link);
- if (err) {
+ if (!link) {
p_err("attach_iter failed for program %s",
bpf_program__name(prog));
goto close_obj;
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 87ceafa4b9b8..da43ba596610 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -510,10 +510,9 @@ int main(int argc, char **argv)
break;
case 'B':
base_btf = btf__parse(optarg, NULL);
- if (libbpf_get_error(base_btf)) {
- p_err("failed to parse base BTF at '%s': %ld\n",
- optarg, libbpf_get_error(base_btf));
- base_btf = NULL;
+ if (!base_btf) {
+ p_err("failed to parse base BTF at '%s': %d\n",
+ optarg, errno);
return -1;
}
break;
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index f941ac5c7b73..b6bddf6c789e 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -788,18 +788,18 @@ static int get_map_kv_btf(const struct bpf_map_info *info, struct btf **btf)
if (info->btf_vmlinux_value_type_id) {
if (!btf_vmlinux) {
btf_vmlinux = libbpf_find_kernel_btf();
- err = libbpf_get_error(btf_vmlinux);
- if (err) {
+ if (!btf_vmlinux) {
p_err("failed to get kernel btf");
- return err;
+ return errno;
}
}
*btf = btf_vmlinux;
} else if (info->btf_value_type_id) {
*btf = btf__load_from_kernel_by_id(info->btf_id);
- err = libbpf_get_error(*btf);
- if (err)
+ if (!*btf) {
+ err = errno;
p_err("failed to get btf");
+ }
} else {
*btf = NULL;
}
@@ -809,14 +809,13 @@ static int get_map_kv_btf(const struct bpf_map_info *info, struct btf **btf)
static void free_map_kv_btf(struct btf *btf)
{
- if (!libbpf_get_error(btf) && btf != btf_vmlinux)
+ if (btf != btf_vmlinux)
btf__free(btf);
}
static void free_btf_vmlinux(void)
{
- if (!libbpf_get_error(btf_vmlinux))
- btf__free(btf_vmlinux);
+ btf__free(btf_vmlinux);
}
static int
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index b6b62b3ef49b..72e1c458dadc 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -322,7 +322,7 @@ static void show_prog_metadata(int fd, __u32 num_maps)
return;
btf = btf__load_from_kernel_by_id(map_info.btf_id);
- if (libbpf_get_error(btf))
+ if (!btf)
goto out_free;
t_datasec = btf__type_by_id(btf, map_info.btf_value_type_id);
@@ -728,7 +728,7 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode,
if (info->btf_id) {
btf = btf__load_from_kernel_by_id(info->btf_id);
- if (libbpf_get_error(btf)) {
+ if (!btf) {
p_err("failed to get btf");
return -1;
}
@@ -1665,7 +1665,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
open_opts.kernel_log_level = 1 + 2 + 4;
obj = bpf_object__open_file(file, &open_opts);
- if (libbpf_get_error(obj)) {
+ if (!obj) {
p_err("failed to open object file");
goto err_free_reuse_maps;
}
@@ -1884,7 +1884,7 @@ static int do_loader(int argc, char **argv)
open_opts.kernel_log_level = 1 + 2 + 4;
obj = bpf_object__open_file(file, &open_opts);
- if (libbpf_get_error(obj)) {
+ if (!obj) {
p_err("failed to open object file");
goto err_close_obj;
}
@@ -2201,7 +2201,7 @@ static char *profile_target_name(int tgt_fd)
}
btf = btf__load_from_kernel_by_id(info.btf_id);
- if (libbpf_get_error(btf)) {
+ if (!btf) {
p_err("failed to load btf for prog FD %d", tgt_fd);
goto out;
}
diff --git a/tools/bpf/bpftool/struct_ops.c b/tools/bpf/bpftool/struct_ops.c
index e0927dbb837b..903b80ff4e9a 100644
--- a/tools/bpf/bpftool/struct_ops.c
+++ b/tools/bpf/bpftool/struct_ops.c
@@ -32,7 +32,7 @@ static const struct btf *get_btf_vmlinux(void)
return btf_vmlinux;
btf_vmlinux = libbpf_find_kernel_btf();
- if (libbpf_get_error(btf_vmlinux))
+ if (!btf_vmlinux)
p_err("struct_ops requires kernel CONFIG_DEBUG_INFO_BTF=y");
return btf_vmlinux;
@@ -45,7 +45,7 @@ static const char *get_kern_struct_ops_name(const struct bpf_map_info *info)
const char *st_ops_name;
kern_btf = get_btf_vmlinux();
- if (libbpf_get_error(kern_btf))
+ if (!kern_btf)
return "<btf_vmlinux_not_found>";
t = btf__type_by_id(kern_btf, info->btf_vmlinux_value_type_id);
@@ -63,7 +63,7 @@ static __s32 get_map_info_type_id(void)
return map_info_type_id;
kern_btf = get_btf_vmlinux();
- if (libbpf_get_error(kern_btf))
+ if (!kern_btf)
return 0;
map_info_type_id = btf__find_by_name_kind(kern_btf, "bpf_map_info",
@@ -413,7 +413,7 @@ static int do_dump(int argc, char **argv)
}
kern_btf = get_btf_vmlinux();
- if (libbpf_get_error(kern_btf))
+ if (!kern_btf)
return -1;
if (!json_output) {
@@ -496,7 +496,7 @@ static int do_register(int argc, char **argv)
open_opts.kernel_log_level = 1 + 2 + 4;
obj = bpf_object__open_file(file, &open_opts);
- if (libbpf_get_error(obj))
+ if (!obj)
return -1;
set_max_rlimit();
@@ -511,7 +511,7 @@ static int do_register(int argc, char **argv)
continue;
link = bpf_map__attach_struct_ops(map);
- if (libbpf_get_error(link)) {
+ if (!link) {
p_err("can't register struct_ops %s: %s",
bpf_map__name(map), strerror(errno));
nr_errs++;
@@ -590,8 +590,7 @@ int do_struct_ops(int argc, char **argv)
err = cmd_select(cmds, argc, argv, do_help);
- if (!libbpf_get_error(btf_vmlinux))
- btf__free(btf_vmlinux);
+ btf__free(btf_vmlinux);
return err;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH bpf-next v2 4/5] bpftool: clean-up usage of libbpf_get_error()
2022-11-09 7:45 [PATCH bpf-next v2 4/5] bpftool: clean-up usage of libbpf_get_error() Sahid Orentino Ferdjaoui
@ 2022-11-10 0:03 ` Andrii Nakryiko
0 siblings, 0 replies; 2+ messages in thread
From: Andrii Nakryiko @ 2022-11-10 0:03 UTC (permalink / raw)
To: Sahid Orentino Ferdjaoui
Cc: bpf, ast, daniel, andrii, quentin, martin.lau, song, yhs,
john.fastabend, kpsingh, sdf, haoluo, jolsa
On Tue, Nov 8, 2022 at 11:46 PM Sahid Orentino Ferdjaoui
<sahid.ferdjaoui@industrialdiscipline.com> wrote:
>
> bpftool is now totally compliant with libbpf 1.0 mode and is not
> expected to be compiled with pre-1.0, let's clean-up the usage of
> libbpf_get_error().
>
> In some places functions that are returning result of
> libbpf_get_error() will now return errno. This won't impact the
> functionality as the caller checks for 0/non-0 for success/failure.
>
> sidenode: We can remove the checks on NULL pointers before calling
> btf__free() because that function already does the check.
>
> Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@industrialdiscipline.com>
> ---
> tools/bpf/bpftool/btf.c | 17 +++++++----------
> tools/bpf/bpftool/btf_dumper.c | 2 +-
> tools/bpf/bpftool/gen.c | 11 ++++-------
> tools/bpf/bpftool/iter.c | 6 ++----
> tools/bpf/bpftool/main.c | 7 +++----
> tools/bpf/bpftool/map.c | 15 +++++++--------
> tools/bpf/bpftool/prog.c | 10 +++++-----
> tools/bpf/bpftool/struct_ops.c | 15 +++++++--------
> 8 files changed, 36 insertions(+), 47 deletions(-)
>
> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index 68a70ac03c80..ed39b24e39d2 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c
> @@ -467,9 +467,8 @@ static int dump_btf_c(const struct btf *btf,
> int err = 0, i;
>
> d = btf_dump__new(btf, btf_dump_printf, NULL, NULL);
> - err = libbpf_get_error(d);
> - if (err)
> - return err;
> + if (!d)
> + return errno;
-errno
>
> printf("#ifndef __VMLINUX_H__\n");
> printf("#define __VMLINUX_H__\n");
> @@ -512,11 +511,9 @@ static struct btf *get_vmlinux_btf_from_sysfs(void)
> struct btf *base;
>
> base = btf__parse(sysfs_vmlinux, NULL);
> - if (libbpf_get_error(base)) {
> - p_err("failed to parse vmlinux BTF at '%s': %ld\n",
> - sysfs_vmlinux, libbpf_get_error(base));
> - base = NULL;
> - }
> + if (!base)
> + p_err("failed to parse vmlinux BTF at '%s': %d\n",
> + sysfs_vmlinux, errno);
to be exactly the same: -errno (errno is always positive, while
returned error is always negative)
>
> return base;
> }
> @@ -634,8 +631,8 @@ static int do_dump(int argc, char **argv)
> base = get_vmlinux_btf_from_sysfs();
>
> btf = btf__parse_split(*argv, base ?: base_btf);
> - err = libbpf_get_error(btf);
you are removing setting err to actual value returned by libbpf. I
don't know if that matters, but it's certainly a change. Now on error
you'll always return -1. Please check if that has any unexpected
effect on calling code.
> if (!btf) {
> + err = errno;
-errno
> p_err("failed to load BTF from %s: %s",
> *argv, strerror(errno));
> goto done;
> @@ -681,8 +678,8 @@ static int do_dump(int argc, char **argv)
> }
>
> btf = btf__load_from_kernel_by_id_split(btf_id, base_btf);
> - err = libbpf_get_error(btf);
> if (!btf) {
> + err = errno;
same
> p_err("get btf by id (%u): %s", btf_id, strerror(errno));
> goto done;
> }
> diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
> index 19924b6ce796..eda71fdfe95a 100644
> --- a/tools/bpf/bpftool/btf_dumper.c
> +++ b/tools/bpf/bpftool/btf_dumper.c
> @@ -75,7 +75,7 @@ static int dump_prog_id_as_func_ptr(const struct btf_dumper *d,
> goto print;
>
> prog_btf = btf__load_from_kernel_by_id(info.btf_id);
> - if (libbpf_get_error(prog_btf))
> + if (!prog_btf)
> goto print;
> func_type = btf__type_by_id(prog_btf, finfo.type_id);
> if (!func_type || !btf_is_func(func_type))
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index cf8b4e525c88..d00b4e1b99ba 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
> @@ -252,9 +252,8 @@ static int codegen_datasecs(struct bpf_object *obj, const char *obj_name)
> int err = 0;
>
> d = btf_dump__new(btf, codegen_btf_dump_printf, NULL, NULL);
> - err = libbpf_get_error(d);
> - if (err)
> - return err;
> + if (!d)
> + return errno;
-errno
>
> bpf_object__for_each_map(map, obj) {
> /* only generate definitions for memory-mapped internal maps */
> @@ -976,13 +975,11 @@ static int do_skeleton(int argc, char **argv)
> /* log_level1 + log_level2 + stats, but not stable UAPI */
> opts.kernel_log_level = 1 + 2 + 4;
> obj = bpf_object__open_mem(obj_data, file_sz, &opts);
> - err = libbpf_get_error(obj);
> - if (err) {
> + if (!obj) {
> char err_buf[256];
>
> - libbpf_strerror(err, err_buf, sizeof(err_buf));
> + libbpf_strerror(errno, err_buf, sizeof(err_buf));
in this case doesn't matter, as libbpf_strerror drops the sign anyway
> p_err("failed to open BPF object file: %s", err_buf);
> - obj = NULL;
> goto out;
> }
>
[...]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-11-10 0:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-09 7:45 [PATCH bpf-next v2 4/5] bpftool: clean-up usage of libbpf_get_error() Sahid Orentino Ferdjaoui
2022-11-10 0:03 ` Andrii Nakryiko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.