linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Florent Revest <revest@chromium.org>
Cc: kernel test robot <lkp@intel.com>,
	kbuild-all@lists.01.org,
	 Linux Memory Management List <linux-mm@kvack.org>,
	Alexei Starovoitov <ast@kernel.org>
Subject: Re: [linux-next:master 2296/2770] kernel/bpf/helpers.c:713:43: warning: Uninitialized variable: bufs [uninitvar]
Date: Fri, 14 May 2021 07:17:59 -0700	[thread overview]
Message-ID: <CAADnVQ+tYSnbgDP-+6vDG+kmJY3ngp2thoWpnUpT1X17VgqaaA@mail.gmail.com> (raw)
In-Reply-To: <CABRcYmLgV-5eDcuZL_sm95FJsVnjFgQ6b7bsA0o_u9bX5+FRnQ@mail.gmail.com>

On Fri, May 14, 2021 at 7:10 AM Florent Revest <revest@chromium.org> wrote:
>
> On Fri, May 14, 2021 at 8:37 AM kernel test robot <lkp@intel.com> 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.


  reply	other threads:[~2021-05-14 14:18 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-14  6:36 kernel test robot
2021-05-14 14:10 ` Florent Revest
2021-05-14 14:17   ` Alexei Starovoitov [this message]
2021-05-17  9:30     ` Florent Revest

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAADnVQ+tYSnbgDP-+6vDG+kmJY3ngp2thoWpnUpT1X17VgqaaA@mail.gmail.com \
    --to=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=kbuild-all@lists.01.org \
    --cc=linux-mm@kvack.org \
    --cc=lkp@intel.com \
    --cc=revest@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox