| Message ID | 20180427140459.GB19583@mwanda |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [1/2] bpf: btf: silence uninitialize variable warnings | expand |
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);
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);
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
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
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
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 >
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 > >
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);
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)