Wed Sep 28 2022 09:09:32 GMT+0100 ~ Tianyi Liu <i.pear@xxxxxxxxxxx> > strerror() expects a positive errno, however variable err will never be > positive when an error occurs. This causes bpftool to output too many > "unknown error", even a simple "file not exist" error can not get an > accurate message. > > This patch fixed all "strerror(err)" patterns in bpftool. > Specially in btf.c#L823, hashmap__append() is an internal function of > libbpf and will not change errno, so there's a little difference. > Some libbpf_get_error() calls are kept for return values. > > Changes since v1: https://lore.kernel.org/bpf/SY4P282MB1084B61CD8671DFA395AA8579D539@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ > Check directly for NULL values instead of calling libbpf_get_error(). > > Signed-off-by: Tianyi Liu <i.pear@xxxxxxxxxxx> > --- > tools/bpf/bpftool/btf.c | 11 +++++------ > tools/bpf/bpftool/gen.c | 4 ++-- > tools/bpf/bpftool/map_perf_ring.c | 7 +++---- > 3 files changed, 10 insertions(+), 12 deletions(-) > > diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c > index 0744bd115..933177bdd 100644 > --- a/tools/bpf/bpftool/btf.c > +++ b/tools/bpf/bpftool/btf.c > @@ -640,10 +640,9 @@ static int do_dump(int argc, char **argv) > > btf = btf__parse_split(*argv, base ?: base_btf); > err = libbpf_get_error(btf); > - if (err) { > - btf = NULL; > + if (!btf) { > p_err("failed to load BTF from %s: %s", > - *argv, strerror(err)); > + *argv, strerror(errno)); > goto done; > } I thought we could get rid of libbpf_get_error() entirely, but I hadn't realised "err" is later returned by the function. We'll need a pass to clean these up eventually, but this is beyond the scope of your patch. Thanks! And good catch for hashmap__append(). Reviewed-by: Quentin Monnet <quentin@xxxxxxxxxxxxx>
