[2/2] bpf: btf: remove a couple conditions
diff mbox series

Message ID 20180427140459.GB19583@mwanda
State Changes Requested, archived
Delegated to: BPF Maintainers
Headers show
Series
  • [1/2] bpf: btf: silence uninitialize variable warnings
Related show

Commit Message

Dan Carpenter April 27, 2018, 2:04 p.m. UTC
We know "err" is zero so we can remove these and pull the code in one
indent level.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This applies to the BPF tree (linux-next)

Comments

Martin Lau April 27, 2018, 5:20 p.m. UTC | #1
On Fri, Apr 27, 2018 at 05:04:59PM +0300, Dan Carpenter wrote:
> We know "err" is zero so we can remove these and pull the code in one
> indent level.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Thanks for the simplification!

Acked-by: Martin KaFai Lau <kafai@fb.com>

> ---
> This applies to the BPF tree (linux-next)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index e631b6fd60d3..7cb0905f37c2 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -1973,16 +1973,14 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
>  	if (err)
>  		goto errout;
>  
> -	if (!err && log->level && bpf_verifier_log_full(log)) {
> +	if (log->level && bpf_verifier_log_full(log)) {
>  		err = -ENOSPC;
>  		goto errout;
>  	}
>  
> -	if (!err) {
> -		btf_verifier_env_free(env);
> -		btf_get(btf);
> -		return btf;
> -	}
> +	btf_verifier_env_free(env);
> +	btf_get(btf);
> +	return btf;
>  
>  errout:
>  	btf_verifier_env_free(env);
Martin Lau April 27, 2018, 5:55 p.m. UTC | #2
On Fri, Apr 27, 2018 at 10:20:25AM -0700, Martin KaFai Lau wrote:
> On Fri, Apr 27, 2018 at 05:04:59PM +0300, Dan Carpenter wrote:
> > We know "err" is zero so we can remove these and pull the code in one
> > indent level.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Thanks for the simplification!
> 
> Acked-by: Martin KaFai Lau <kafai@fb.com>
btw, it should be for bpf-next.  Please tag the subject with bpf-next when
you respin. Thanks!

> 
> > ---
> > This applies to the BPF tree (linux-next)
> > 
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index e631b6fd60d3..7cb0905f37c2 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -1973,16 +1973,14 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
> >  	if (err)
> >  		goto errout;
> >  
> > -	if (!err && log->level && bpf_verifier_log_full(log)) {
> > +	if (log->level && bpf_verifier_log_full(log)) {
> >  		err = -ENOSPC;
> >  		goto errout;
> >  	}
> >  
> > -	if (!err) {
> > -		btf_verifier_env_free(env);
> > -		btf_get(btf);
> > -		return btf;
> > -	}
> > +	btf_verifier_env_free(env);
> > +	btf_get(btf);
> > +	return btf;
> >  
> >  errout:
> >  	btf_verifier_env_free(env);
Dan Carpenter April 27, 2018, 7:39 p.m. UTC | #3
On Fri, Apr 27, 2018 at 10:55:46AM -0700, Martin KaFai Lau wrote:
> On Fri, Apr 27, 2018 at 10:20:25AM -0700, Martin KaFai Lau wrote:
> > On Fri, Apr 27, 2018 at 05:04:59PM +0300, Dan Carpenter wrote:
> > > We know "err" is zero so we can remove these and pull the code in one
> > > indent level.
> > > 
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Thanks for the simplification!
> > 
> > Acked-by: Martin KaFai Lau <kafai@fb.com>
> btw, it should be for bpf-next.  Please tag the subject with bpf-next when
> you respin. Thanks!
>

I'm working against linux-next.  For networking, I have a separate tree
which I use to figure out if it's in net or net-next.  It's kind of a
headache (but obviously networking is the largest subtree so it's
required).

Is there an automated way to tie a Fixes tag from linux-next to a
subtree?

regards,
dan carpenter
Daniel Borkmann April 27, 2018, 8:21 p.m. UTC | #4
On 04/27/2018 09:39 PM, Dan Carpenter wrote:
> On Fri, Apr 27, 2018 at 10:55:46AM -0700, Martin KaFai Lau wrote:
>> On Fri, Apr 27, 2018 at 10:20:25AM -0700, Martin KaFai Lau wrote:
>>> On Fri, Apr 27, 2018 at 05:04:59PM +0300, Dan Carpenter wrote:
>>>> We know "err" is zero so we can remove these and pull the code in one
>>>> indent level.
>>>>
>>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>> Thanks for the simplification!
>>>
>>> Acked-by: Martin KaFai Lau <kafai@fb.com>
>> btw, it should be for bpf-next.  Please tag the subject with bpf-next when
>> you respin. Thanks!

Dan, thanks a lot for your fixes! Please respin with addressing Martin's
feedback when you get a chance.

Thanks,
Daniel
Dan Carpenter April 27, 2018, 8:31 p.m. UTC | #5
On Fri, Apr 27, 2018 at 10:21:17PM +0200, Daniel Borkmann wrote:
> On 04/27/2018 09:39 PM, Dan Carpenter wrote:
> > On Fri, Apr 27, 2018 at 10:55:46AM -0700, Martin KaFai Lau wrote:
> >> On Fri, Apr 27, 2018 at 10:20:25AM -0700, Martin KaFai Lau wrote:
> >>> On Fri, Apr 27, 2018 at 05:04:59PM +0300, Dan Carpenter wrote:
> >>>> We know "err" is zero so we can remove these and pull the code in one
> >>>> indent level.
> >>>>
> >>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >>> Thanks for the simplification!
> >>>
> >>> Acked-by: Martin KaFai Lau <kafai@fb.com>
> >> btw, it should be for bpf-next.  Please tag the subject with bpf-next when
> >> you respin. Thanks!
> 
> Dan, thanks a lot for your fixes! Please respin with addressing Martin's
> feedback when you get a chance.
> 

My understanding is that he'd prefer we just ignore the static checker
warning since it's a false positive.  Should I instead initialize the
size to zero or something just to silence it?

regards,
dan carpenter
Martin Lau April 27, 2018, 9:26 p.m. UTC | #6
On Fri, Apr 27, 2018 at 11:31:36PM +0300, Dan Carpenter wrote:
> On Fri, Apr 27, 2018 at 10:21:17PM +0200, Daniel Borkmann wrote:
> > On 04/27/2018 09:39 PM, Dan Carpenter wrote:
> > > On Fri, Apr 27, 2018 at 10:55:46AM -0700, Martin KaFai Lau wrote:
> > >> On Fri, Apr 27, 2018 at 10:20:25AM -0700, Martin KaFai Lau wrote:
> > >>> On Fri, Apr 27, 2018 at 05:04:59PM +0300, Dan Carpenter wrote:
> > >>>> We know "err" is zero so we can remove these and pull the code in one
> > >>>> indent level.
> > >>>>
> > >>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > >>> Thanks for the simplification!
> > >>>
> > >>> Acked-by: Martin KaFai Lau <kafai@fb.com>
> > >> btw, it should be for bpf-next.  Please tag the subject with bpf-next when
> > >> you respin. Thanks!
> > 
> > Dan, thanks a lot for your fixes! Please respin with addressing Martin's
> > feedback when you get a chance.
> > 
> 
> My understanding is that he'd prefer we just ignore the static checker
> warning since it's a false positive.
Right, I think patch 1 is not needed.  I would prefer to use a comment
in those cases.

> Should I instead initialize the
> size to zero or something just to silence it?
> 
> regards,
> dan carpenter
>
Martin Lau April 28, 2018, 1:27 a.m. UTC | #7
On Fri, Apr 27, 2018 at 02:26:50PM -0700, Martin KaFai Lau wrote:
> On Fri, Apr 27, 2018 at 11:31:36PM +0300, Dan Carpenter wrote:
> > On Fri, Apr 27, 2018 at 10:21:17PM +0200, Daniel Borkmann wrote:
> > > On 04/27/2018 09:39 PM, Dan Carpenter wrote:
> > > > On Fri, Apr 27, 2018 at 10:55:46AM -0700, Martin KaFai Lau wrote:
> > > >> On Fri, Apr 27, 2018 at 10:20:25AM -0700, Martin KaFai Lau wrote:
> > > >>> On Fri, Apr 27, 2018 at 05:04:59PM +0300, Dan Carpenter wrote:
> > > >>>> We know "err" is zero so we can remove these and pull the code in one
> > > >>>> indent level.
> > > >>>>
> > > >>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > >>> Thanks for the simplification!
> > > >>>
> > > >>> Acked-by: Martin KaFai Lau <kafai@fb.com>
> > > >> btw, it should be for bpf-next.  Please tag the subject with bpf-next when
> > > >> you respin. Thanks!
> > > 
> > > Dan, thanks a lot for your fixes! Please respin with addressing Martin's
> > > feedback when you get a chance.
> > > 
> > 
> > My understanding is that he'd prefer we just ignore the static checker
> > warning since it's a false positive.
> Right, I think patch 1 is not needed.  I would prefer to use a comment
> in those cases.
> 
> > Should I instead initialize the
> > size to zero or something just to silence it?
After another thought,  I think init size to zero is
fine which is less intrusive.

Thanks!
Martin

> > 
> > regards,
> > dan carpenter
> >

Patch
diff mbox series

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index e631b6fd60d3..7cb0905f37c2 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -1973,16 +1973,14 @@  static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
 	if (err)
 		goto errout;
 
-	if (!err && log->level && bpf_verifier_log_full(log)) {
+	if (log->level && bpf_verifier_log_full(log)) {
 		err = -ENOSPC;
 		goto errout;
 	}
 
-	if (!err) {
-		btf_verifier_env_free(env);
-		btf_get(btf);
-		return btf;
-	}
+	btf_verifier_env_free(env);
+	btf_get(btf);
+	return btf;
 
 errout:
 	btf_verifier_env_free(env);