| Message ID | 20190815143220.4199-1-quentin.monnet@netronome.com |
|---|---|
| Headers | show |
| Series | tools: bpftool: fix printf()-like functions | expand |
On Thu, Aug 15, 2019 at 7:32 AM Quentin Monnet <quentin.monnet@netronome.com> wrote: > > Hi, > Because the "__printf()" attributes were used only where the functions are > implemented, and not in header files, the checks have not been enforced on > all the calls to printf()-like functions, and a number of errors slipped in > bpftool over time. > > This set cleans up such errors, and then moves the "__printf()" attributes > to header files, so that the checks are performed at all locations. Applied. Thanks
2019-08-15 22:08 UTC-0700 ~ Alexei Starovoitov <alexei.starovoitov@gmail.com> > On Thu, Aug 15, 2019 at 7:32 AM Quentin Monnet > <quentin.monnet@netronome.com> wrote: >> >> Hi, >> Because the "__printf()" attributes were used only where the functions are >> implemented, and not in header files, the checks have not been enforced on >> all the calls to printf()-like functions, and a number of errors slipped in >> bpftool over time. >> >> This set cleans up such errors, and then moves the "__printf()" attributes >> to header files, so that the checks are performed at all locations. > > Applied. Thanks > Thanks Alexei! I noticed the set was applied to the bpf-next tree, and not bpf. Just checking if this is intentional? Regards, Quentin
On Fri, Aug 16, 2019 at 9:41 AM Quentin Monnet <quentin.monnet@netronome.com> wrote: > > 2019-08-15 22:08 UTC-0700 ~ Alexei Starovoitov > <alexei.starovoitov@gmail.com> > > On Thu, Aug 15, 2019 at 7:32 AM Quentin Monnet > > <quentin.monnet@netronome.com> wrote: > >> > >> Hi, > >> Because the "__printf()" attributes were used only where the functions are > >> implemented, and not in header files, the checks have not been enforced on > >> all the calls to printf()-like functions, and a number of errors slipped in > >> bpftool over time. > >> > >> This set cleans up such errors, and then moves the "__printf()" attributes > >> to header files, so that the checks are performed at all locations. > > > > Applied. Thanks > > > > Thanks Alexei! > > I noticed the set was applied to the bpf-next tree, and not bpf. Just > checking if this is intentional? Yes. I don't see the _fix_ part in there. Looks like cleanup to me. I've also considered to push commit d34b044038bf ("tools: bpftool: close prog FD before exit on showing a single program") to bpf-next as well. That fd leak didn't feel that necessary to push to bpf tree and risk merge conflicts... but I pushed it to bpf at the end.
2019-08-16 10:11 UTC-0700 ~ Alexei Starovoitov <alexei.starovoitov@gmail.com> > On Fri, Aug 16, 2019 at 9:41 AM Quentin Monnet > <quentin.monnet@netronome.com> wrote: >> >> 2019-08-15 22:08 UTC-0700 ~ Alexei Starovoitov >> <alexei.starovoitov@gmail.com> >>> On Thu, Aug 15, 2019 at 7:32 AM Quentin Monnet >>> <quentin.monnet@netronome.com> wrote: >>>> >>>> Hi, >>>> Because the "__printf()" attributes were used only where the functions are >>>> implemented, and not in header files, the checks have not been enforced on >>>> all the calls to printf()-like functions, and a number of errors slipped in >>>> bpftool over time. >>>> >>>> This set cleans up such errors, and then moves the "__printf()" attributes >>>> to header files, so that the checks are performed at all locations. >>> >>> Applied. Thanks >>> >> >> Thanks Alexei! >> >> I noticed the set was applied to the bpf-next tree, and not bpf. Just >> checking if this is intentional? > > Yes. I don't see the _fix_ part in there. > Looks like cleanup to me. > I've also considered to push > commit d34b044038bf ("tools: bpftool: close prog FD before exit on > showing a single program") > to bpf-next as well. > That fd leak didn't feel that necessary to push to bpf tree > and risk merge conflicts... but I pushed it to bpf at the end. > Ok, thanks for explaining. I'll consider submitting this kind of patches to bpf-next instead in the future. Quentin
On Fri, 16 Aug 2019 10:11:12 -0700, Alexei Starovoitov wrote: > On Fri, Aug 16, 2019 at 9:41 AM Quentin Monnet wrote: > > 2019-08-15 22:08 UTC-0700 ~ Alexei Starovoitov > > <alexei.starovoitov@gmail.com> > > > On Thu, Aug 15, 2019 at 7:32 AM Quentin Monnet > > > <quentin.monnet@netronome.com> wrote: > > >> > > >> Hi, > > >> Because the "__printf()" attributes were used only where the functions are > > >> implemented, and not in header files, the checks have not been enforced on > > >> all the calls to printf()-like functions, and a number of errors slipped in > > >> bpftool over time. > > >> > > >> This set cleans up such errors, and then moves the "__printf()" attributes > > >> to header files, so that the checks are performed at all locations. > > > > > > Applied. Thanks > > > > > > > Thanks Alexei! > > > > I noticed the set was applied to the bpf-next tree, and not bpf. Just > > checking if this is intentional? > > Yes. I don't see the _fix_ part in there. Mm.. these are not critical indeed, but patches 1 and 3 do fix a crash. Perhaps those should had been a series on their own. We'll recalibrate :) > Looks like cleanup to me. > I've also considered to push > commit d34b044038bf ("tools: bpftool: close prog FD before exit on > showing a single program") > to bpf-next as well. > That fd leak didn't feel that necessary to push to bpf tree > and risk merge conflicts... but I pushed it to bpf at the end.
Hi, Because the "__printf()" attributes were used only where the functions are implemented, and not in header files, the checks have not been enforced on all the calls to printf()-like functions, and a number of errors slipped in bpftool over time. This set cleans up such errors, and then moves the "__printf()" attributes to header files, so that the checks are performed at all locations. Quentin Monnet (6): tools: bpftool: fix arguments for p_err() in do_event_pipe() tools: bpftool: fix format strings and arguments for jsonw_printf() tools: bpftool: fix argument for p_err() in BTF do_dump() tools: bpftool: fix format string for p_err() in query_flow_dissector() tools: bpftool: fix format string for p_err() in detect_common_prefix() tools: bpftool: move "__printf()" attributes to header file tools/bpf/bpftool/btf.c | 2 +- tools/bpf/bpftool/btf_dumper.c | 8 ++++---- tools/bpf/bpftool/common.c | 4 ++-- tools/bpf/bpftool/json_writer.c | 6 ++---- tools/bpf/bpftool/json_writer.h | 6 ++++-- tools/bpf/bpftool/main.c | 2 +- tools/bpf/bpftool/main.h | 4 ++-- tools/bpf/bpftool/map_perf_ring.c | 4 ++-- tools/bpf/bpftool/net.c | 2 +- tools/include/linux/compiler-gcc.h | 2 ++ 10 files changed, 21 insertions(+), 19 deletions(-)