| Message ID | 20250214141717.26847-1-chen.dylane@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | libbpf: Wrap libbpf API direct err with libbpf_err | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
在 2025/2/14 22:17, Tao Chen 写道: > Just wrap the direct err with libbpf_err, keep consistency > with other APIs. > > Signed-off-by: Tao Chen <chen.dylane@gmail.com> > --- > tools/lib/bpf/libbpf.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 194809da5172..6f2f3072f5a2 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -9145,12 +9145,12 @@ int bpf_object__gen_loader(struct bpf_object *obj, struct gen_loader_opts *opts) > struct bpf_gen *gen; > > if (!opts) > - return -EFAULT; > + return libbpf_err(-EFAULT); > if (!OPTS_VALID(opts, gen_loader_opts)) > - return -EINVAL; > + return libbpf_err(-EINVAL); > gen = calloc(sizeof(*gen), 1); > if (!gen) > - return -ENOMEM; > + return libbpf_err(-ENOMEM); > gen->opts = opts; > gen->swapped_endian = !is_native_endianness(obj); > obj->gen_loader = gen; > @@ -9262,13 +9262,13 @@ int bpf_program__set_insns(struct bpf_program *prog, > struct bpf_insn *insns; > > if (prog->obj->loaded) > - return -EBUSY; > + return libbpf_err(-EBUSY); > > insns = libbpf_reallocarray(prog->insns, new_insn_cnt, sizeof(*insns)); > /* NULL is a valid return from reallocarray if the new count is zero */ > if (!insns && new_insn_cnt) { > pr_warn("prog '%s': failed to realloc prog code\n", prog->name); > - return -ENOMEM; > + return libbpf_err(-ENOMEM); > } > memcpy(insns, new_insns, new_insn_cnt * sizeof(*insns)); > > @@ -9379,11 +9379,11 @@ const char *bpf_program__log_buf(const struct bpf_program *prog, size_t *log_siz > int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log_size) > { > if (log_size && !log_buf) > - return -EINVAL; > + return libbpf_err(-EINVAL); > if (prog->log_size > UINT_MAX) > - return -EINVAL; > + return libbpf_err(-EINVAL); > if (prog->obj->loaded) > - return -EBUSY; > + return libbpf_err(-EBUSY); > > prog->log_buf = log_buf; > prog->log_size = log_size; > @@ -13070,17 +13070,17 @@ int bpf_link__update_map(struct bpf_link *link, const struct bpf_map *map) > int err; > > if (!bpf_map__is_struct_ops(map)) > - return -EINVAL; > + return libbpf_err(-EINVAL); > > if (map->fd < 0) { > pr_warn("map '%s': can't use BPF map without FD (was it created?)\n", map->name); > - return -EINVAL; > + return libbpf_err(-EINVAL); > } > > st_ops_link = container_of(link, struct bpf_link_struct_ops, link); > /* Ensure the type of a link is correct */ > if (st_ops_link->map_fd < 0) > - return -EINVAL; > + return libbpf_err(-EINVAL); > > err = bpf_map_update_elem(map->fd, &zero, map->st_ops->kern_vdata, 0); > /* It can be EBUSY if the map has been used to create or Forgive my carelessness. This patch was developed based on bpf-next. You can ignore it. I've resent it.
On Fri, 2025-02-14 at 22:17 +0800, Tao Chen wrote: > Just wrap the direct err with libbpf_err, keep consistency > with other APIs. > > Signed-off-by: Tao Chen <chen.dylane@gmail.com> > --- While at it, I've noticed two more places that need libbpf_err() calls. Could you please check the following locations: bpf_map__set_value_size: return -EOPNOTSUPP; tools/lib/bpf/libbpf.c:10309 return err; tools/lib/bpf/libbpf.c:10317 ? Other than that, I agree with changes in this patch. Acked-by: Eduard Zingerman <eddyz87@gmail.com> [...]
在 2025/2/19 10:08, Eduard Zingerman 写道: > On Fri, 2025-02-14 at 22:17 +0800, Tao Chen wrote: >> Just wrap the direct err with libbpf_err, keep consistency >> with other APIs. >> >> Signed-off-by: Tao Chen <chen.dylane@gmail.com> >> --- > > While at it, I've noticed two more places that need libbpf_err() calls. > Could you please check the following locations: > > bpf_map__set_value_size: > return -EOPNOTSUPP; tools/lib/bpf/libbpf.c:10309 > return err; tools/lib/bpf/libbpf.c:10317 Will change it. Thanks > > ? > > Other than that, I agree with changes in this patch. > > Acked-by: Eduard Zingerman <eddyz87@gmail.com> > > [...] > I use a simple script, other places may also should be added: 9727 line: return NUL; (API:libbpf_bpf_attach_type_str) 9735 line: return NULL; (API: libbpf_bpf_link_type_str) 9743 line: return NULL; (API: libbpf_bpf_map_type_str) 9751 line: return NULL; (API: libbpf_bpf_prog_type_str) 10151 line: return NULL; (API: bpf_map__name) 10458 line: return NULL; (API: bpf_object__prev_map) ----------------------------------------------- #!/bin/python3 import re LIBBPF_PATH="tools/lib/bpf/" def process_functions(file_path, target_functions): """ find return err fix Args: file_path: like libbpf.c btf.c etc. target_functions: libbpf APIS """ with open(file_path, 'r') as file: lines = file.readlines() function_info = {func: {'start': None, 'end': None, 'code': []} for func in target_functions} in_target_function = False current_function = None for i, line in enumerate(lines): # check the start line for func in target_functions: if re.search(re.escape(func) + r'\s*\(', line): if ' __' in line: continue if '=' in line: continue if 'if' in line: continue if ';' in line: continue if '*' in func: continue in_target_function = True current_function = func function_info[func]['start'] = i function_info[func]['code'].append(line) break # check return err in target function if in_target_function and current_function: function_info[current_function]['code'].append(line) if re.search(r'\breturn\b', line): # check return if not re.search(r'\breturn\s+0\b', line): # return 0 ignore if not re.search(r'libbpf_err|libbpf_ptr', line): print(f"code should fix at {i + 1} line: {line.strip()} (API: {current_function})") # check function end line if re.search(r'}', line): function_info[current_function]['end'] = i in_target_function = False current_function = None # Reset for the next function def extract_libbpf_apis(map_file): """ extract APIS from libbpf.map Args: map_file (str): libbpf.map。 Returns: list: libbpf APIs。 """ functions = [] inside_global_section = False with open(map_file, 'r') as f: for line in f: line = line.strip() if ';' in line and 'LIBBPF' not in line: functions.append(line.replace(';','')) return functions map_file = "tools/lib/bpf/libbpf.map" input_files = ["libbpf.c", "btf.c", "bpf_prog_linfo.c", "btf_dump.c", "btf_iter.c", "btf_relocate.c", "elf.c", "features.c", "gen_loader.c", "hashmap.c", "libbpf_probes.c", "linker.c", "netlink.c", "nlattr.c", "relo_core.c", "usdt.c", "zip.c", "str_error.c", "strset.c"] target_functions = extract_libbpf_apis(map_file) print(f"Target functions extracted: {target_functions}") for input_file in input_files: print(f"===========check file:{input_file}=============") process_functions(LIBBPF_PATH + input_file, target_functions)
On Wed, 2025-02-19 at 14:23 +0800, Tao Chen wrote: > 在 2025/2/19 10:08, Eduard Zingerman 写道: > > On Fri, 2025-02-14 at 22:17 +0800, Tao Chen wrote: > > > Just wrap the direct err with libbpf_err, keep consistency > > > with other APIs. > > > > > > Signed-off-by: Tao Chen <chen.dylane@gmail.com> > > > --- > > > > While at it, I've noticed two more places that need libbpf_err() calls. > > Could you please check the following locations: > > > > bpf_map__set_value_size: > > return -EOPNOTSUPP; tools/lib/bpf/libbpf.c:10309 > > return err; tools/lib/bpf/libbpf.c:10317 > > Will change it. Thanks > > > > > ? > > > > Other than that, I agree with changes in this patch. > > > > Acked-by: Eduard Zingerman <eddyz87@gmail.com> > > > > [...] > > > > I use a simple script, other places may also should be added: Yeah, makes sense :) > > 9727 line: return NUL; (API:libbpf_bpf_attach_type_str) > 9735 line: return NULL; (API: libbpf_bpf_link_type_str) > 9743 line: return NULL; (API: libbpf_bpf_map_type_str) > 9751 line: return NULL; (API: libbpf_bpf_prog_type_str) > 10151 line: return NULL; (API: bpf_map__name) Sort of makes sense for these. Idk, I'm fine with and without changes to these functions. > 10458 line: return NULL; (API: bpf_object__prev_map) This is not an error, I think. [...]
在 2025/2/19 14:34, Eduard Zingerman 写道: > On Wed, 2025-02-19 at 14:23 +0800, Tao Chen wrote: >> 在 2025/2/19 10:08, Eduard Zingerman 写道: >>> On Fri, 2025-02-14 at 22:17 +0800, Tao Chen wrote: >>>> Just wrap the direct err with libbpf_err, keep consistency >>>> with other APIs. >>>> >>>> Signed-off-by: Tao Chen <chen.dylane@gmail.com> >>>> --- >>> >>> While at it, I've noticed two more places that need libbpf_err() calls. >>> Could you please check the following locations: >>> >>> bpf_map__set_value_size: >>> return -EOPNOTSUPP; tools/lib/bpf/libbpf.c:10309 >>> return err; tools/lib/bpf/libbpf.c:10317 >> >> Will change it. Thanks >> >>> >>> ? >>> >>> Other than that, I agree with changes in this patch. >>> >>> Acked-by: Eduard Zingerman <eddyz87@gmail.com> >>> >>> [...] >>> >> >> I use a simple script, other places may also should be added: > > Yeah, makes sense :) > >> >> 9727 line: return NUL; (API:libbpf_bpf_attach_type_str) >> 9735 line: return NULL; (API: libbpf_bpf_link_type_str) >> 9743 line: return NULL; (API: libbpf_bpf_map_type_str) >> 9751 line: return NULL; (API: libbpf_bpf_prog_type_str) >> 10151 line: return NULL; (API: bpf_map__name) > > Sort of makes sense for these. > Idk, I'm fine with and without changes to these functions. > Well，then I'll still keep these and won't make any modifications. >> 10458 line: return NULL; (API: bpf_object__prev_map) > > This is not an error, I think. > > [...] >
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 194809da5172..6f2f3072f5a2 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -9145,12 +9145,12 @@ int bpf_object__gen_loader(struct bpf_object *obj, struct gen_loader_opts *opts) struct bpf_gen *gen; if (!opts) - return -EFAULT; + return libbpf_err(-EFAULT); if (!OPTS_VALID(opts, gen_loader_opts)) - return -EINVAL; + return libbpf_err(-EINVAL); gen = calloc(sizeof(*gen), 1); if (!gen) - return -ENOMEM; + return libbpf_err(-ENOMEM); gen->opts = opts; gen->swapped_endian = !is_native_endianness(obj); obj->gen_loader = gen; @@ -9262,13 +9262,13 @@ int bpf_program__set_insns(struct bpf_program *prog, struct bpf_insn *insns; if (prog->obj->loaded) - return -EBUSY; + return libbpf_err(-EBUSY); insns = libbpf_reallocarray(prog->insns, new_insn_cnt, sizeof(*insns)); /* NULL is a valid return from reallocarray if the new count is zero */ if (!insns && new_insn_cnt) { pr_warn("prog '%s': failed to realloc prog code\n", prog->name); - return -ENOMEM; + return libbpf_err(-ENOMEM); } memcpy(insns, new_insns, new_insn_cnt * sizeof(*insns)); @@ -9379,11 +9379,11 @@ const char *bpf_program__log_buf(const struct bpf_program *prog, size_t *log_siz int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log_size) { if (log_size && !log_buf) - return -EINVAL; + return libbpf_err(-EINVAL); if (prog->log_size > UINT_MAX) - return -EINVAL; + return libbpf_err(-EINVAL); if (prog->obj->loaded) - return -EBUSY; + return libbpf_err(-EBUSY); prog->log_buf = log_buf; prog->log_size = log_size; @@ -13070,17 +13070,17 @@ int bpf_link__update_map(struct bpf_link *link, const struct bpf_map *map) int err; if (!bpf_map__is_struct_ops(map)) - return -EINVAL; + return libbpf_err(-EINVAL); if (map->fd < 0) { pr_warn("map '%s': can't use BPF map without FD (was it created?)\n", map->name); - return -EINVAL; + return libbpf_err(-EINVAL); } st_ops_link = container_of(link, struct bpf_link_struct_ops, link); /* Ensure the type of a link is correct */ if (st_ops_link->map_fd < 0) - return -EINVAL; + return libbpf_err(-EINVAL); err = bpf_map_update_elem(map->fd, &zero, map->st_ops->kern_vdata, 0); /* It can be EBUSY if the map has been used to create or
Just wrap the direct err with libbpf_err, keep consistency with other APIs. Signed-off-by: Tao Chen <chen.dylane@gmail.com> --- tools/lib/bpf/libbpf.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)