All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Yonghong Song <yhs@fb.com>, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andriin@fb.com>,
	Daniel Borkmann <daniel@iogearbox.net>, Martin Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"kernel-janitors@vger.kernel.org"
	<kernel-janitors@vger.kernel.org>
Subject: Re: [PATCH] tools/libbpf: signedness bug in btf_dedup_ref_type()
Date: Thu, 28 Feb 2019 09:44:45 -0800	[thread overview]
Message-ID: <CAEf4Bzav1hBca6gQdxJH=TD33AP13d1V9xij8qP8TLWvQLkN5A@mail.gmail.com> (raw)
In-Reply-To: <20190228133605.GC9806@kadam>

On Thu, Feb 28, 2019 at 5:37 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Thu, Feb 28, 2019 at 06:58:29AM +0000, Yonghong Song wrote:
> >
> >
> > On 2/27/19 9:46 PM, Dan Carpenter wrote:
> > > The "ref_type_id" variable needs to be signed for the error handling
> > > to work.
> > >
> > > Fixes: d5caef5b5655 ("btf: add BTF types deduplication algorithm")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > > This goes through the BPF tree probably, although it does apply to
> > > net-next.
> > >
> > >   tools/lib/bpf/btf.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > > index 68b50e9bbde1..8faed5386124 100644
> > > --- a/tools/lib/bpf/btf.c
> > > +++ b/tools/lib/bpf/btf.c
> > > @@ -2326,7 +2326,8 @@ static int btf_dedup_ref_type(struct btf_dedup *d, __u32 type_id)
> > >     struct btf_type *t, *cand;
> > >     /* if we don't find equivalent type, then we are representative type */
> > >     __u32 new_id = type_id;
> > > -   __u32 h, ref_type_id;
> > > +   __u32 h; > +    int ref_type_id;
> >
> > nit. Maybe you can change to
> >       int ref_type_id;
> >       __u32 h;
> > to maintain kernel declaration reverse christmas tree convention?
> >
>
> Yes.  Thanks.  Let me resend.
>

Thanks for catching and fixing this! With Yonghong's suggestion, you can add:

Acked-by: Andrii Nakryiko <andriin@fb.com>


> regards,
> dan carpenter
>

WARNING: multiple messages have this Message-ID (diff)
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Yonghong Song <yhs@fb.com>, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andriin@fb.com>,
	Daniel Borkmann <daniel@iogearbox.net>, Martin Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"kernel-janitors@vger.kernel.org"
	<kernel-janitors@vger.kernel.org>
Subject: Re: [PATCH] tools/libbpf: signedness bug in btf_dedup_ref_type()
Date: Thu, 28 Feb 2019 17:44:45 +0000	[thread overview]
Message-ID: <CAEf4Bzav1hBca6gQdxJH=TD33AP13d1V9xij8qP8TLWvQLkN5A@mail.gmail.com> (raw)
In-Reply-To: <20190228133605.GC9806@kadam>

On Thu, Feb 28, 2019 at 5:37 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Thu, Feb 28, 2019 at 06:58:29AM +0000, Yonghong Song wrote:
> >
> >
> > On 2/27/19 9:46 PM, Dan Carpenter wrote:
> > > The "ref_type_id" variable needs to be signed for the error handling
> > > to work.
> > >
> > > Fixes: d5caef5b5655 ("btf: add BTF types deduplication algorithm")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > > This goes through the BPF tree probably, although it does apply to
> > > net-next.
> > >
> > >   tools/lib/bpf/btf.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > > index 68b50e9bbde1..8faed5386124 100644
> > > --- a/tools/lib/bpf/btf.c
> > > +++ b/tools/lib/bpf/btf.c
> > > @@ -2326,7 +2326,8 @@ static int btf_dedup_ref_type(struct btf_dedup *d, __u32 type_id)
> > >     struct btf_type *t, *cand;
> > >     /* if we don't find equivalent type, then we are representative type */
> > >     __u32 new_id = type_id;
> > > -   __u32 h, ref_type_id;
> > > +   __u32 h; > +    int ref_type_id;
> >
> > nit. Maybe you can change to
> >       int ref_type_id;
> >       __u32 h;
> > to maintain kernel declaration reverse christmas tree convention?
> >
>
> Yes.  Thanks.  Let me resend.
>

Thanks for catching and fixing this! With Yonghong's suggestion, you can add:

Acked-by: Andrii Nakryiko <andriin@fb.com>


> regards,
> dan carpenter
>

  reply	other threads:[~2019-02-28 17:45 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-28  5:46 [PATCH] tools/libbpf: signedness bug in btf_dedup_ref_type() Dan Carpenter
2019-02-28  5:46 ` Dan Carpenter
2019-02-28  6:58 ` Yonghong Song
2019-02-28  6:58   ` Yonghong Song
2019-02-28 13:36   ` Dan Carpenter
2019-02-28 13:36     ` Dan Carpenter
2019-02-28 17:44     ` Andrii Nakryiko [this message]
2019-02-28 17:44       ` Andrii Nakryiko
2019-02-28 17:50   ` [PATCH v2] " Dan Carpenter
2019-02-28 17:50     ` Dan Carpenter
2019-02-28 17:53     ` Arnaldo Carvalho de Melo
2019-02-28 17:53       ` Arnaldo Carvalho de Melo
2019-02-28 18:01       ` Dan Carpenter
2019-02-28 18:01         ` Dan Carpenter
2019-02-28 18:06   ` [PATCH v3] " Dan Carpenter
2019-02-28 18:06     ` Dan Carpenter
2019-02-28 19:29     ` Song Liu
2019-03-01  0:01     ` Daniel Borkmann
2019-03-01  0:01       ` Daniel Borkmann

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='CAEf4Bzav1hBca6gQdxJH=TD33AP13d1V9xij8qP8TLWvQLkN5A@mail.gmail.com' \
    --to=andrii.nakryiko@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@kernel.org \
    --cc=dan.carpenter@oracle.com \
    --cc=daniel@iogearbox.net \
    --cc=kafai@fb.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.