On Fri, May 14, 2021 at 4:18 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Fri, May 14, 2021 at 7:10 AM Florent Revest <revest@xxxxxxxxxxxx> wrote:
> >
> > On Fri, May 14, 2021 at 8:37 AM kernel test robot <lkp@xxxxxxxxx> wrote:
> > > cppcheck possible warnings: (new ones prefixed by >>, may not real problems)
> > >
> > > >> kernel/bpf/helpers.c:713:43: warning: Uninitialized variable: bufs [uninitvar]
> > > if (WARN_ON_ONCE(nest_level > ARRAY_SIZE(bufs->tmp_bufs))) {
> > > ^
> >
> > I don't think this is a real problem. bufs is not actually
> > dereferenced, it is only used to give the type information to a
> > sizeof. This is only evaluated at compilation time.
> >
> > If this matters, I guess we could silent this cppcheck warning with
> > something like the following patch. Alexei, what do you think ?
> >
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -697,8 +697,9 @@ static int bpf_trace_copy_string(char *buf, void
> > *unsafe_ptr, char fmt_ptype,
> > #define MAX_PRINTF_BUF_LEN 512
> >
> > /* Support executing three nested bprintf helper calls on a given CPU */
> > +#define MAX_PRINTF_NEST_LEVEL 3
> > struct bpf_bprintf_buffers {
> > - char tmp_bufs[3][MAX_PRINTF_BUF_LEN];
> > + char tmp_bufs[MAX_PRINTF_NEST_LEVEL][MAX_PRINTF_BUF_LEN];
> > };
> > static DEFINE_PER_CPU(struct bpf_bprintf_buffers, bpf_bprintf_bufs);
> > static DEFINE_PER_CPU(int, bpf_bprintf_nest_level);
> > @@ -710,7 +711,7 @@ static int try_get_fmt_tmp_buf(char **tmp_buf)
> >
> > preempt_disable();
> > nest_level = this_cpu_inc_return(bpf_bprintf_nest_level);
> > - if (WARN_ON_ONCE(nest_level > ARRAY_SIZE(bufs->tmp_bufs))) {
> > + if (WARN_ON_ONCE(nest_level > MAX_PRINTF_NEST_LEVEL)) {
>
> Yeah. Why not. I think it's cleaner overall.
Cool :) thanks, I sent something to the bpf list
