| Message ID | 20170503214411.14270-1-xypron.glpk@gmx.de |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
On 05/03/2017 11:44 PM, Heinrich Schuchardt wrote: > The logical expression to check the dtb is incorrect in > load_devicetree. > > The problem was indicated by cppcheck. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > I do not have a board for testing. > Please, review carefully. > --- > board/BuR/common/common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/board/BuR/common/common.c b/board/BuR/common/common.c > index 876150402c..c0316b9ebd 100644 > --- a/board/BuR/common/common.c > +++ b/board/BuR/common/common.c > @@ -265,7 +265,7 @@ static int load_devicetree(void) > char *dtbname = getenv("dtb"); > char *dtbdev = getenv("dtbdev"); > char *dtppart = getenv("dtbpart"); > - if (!dtbdev || !dtbdev || !dtbname) { > + if (!dtbdev || !dtbpart || !dtbname) { > printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__); > return -1; > } Reviewed-by: Hannes Schmelzer <oe5hpm@oevsv.at> Acked-by: Hannes Schmelzer <oe5hpm@oevsv.at> Thanks!
On Wed, May 03, 2017 at 11:44:11PM +0200, xypron.glpk@gmx.de wrote: > The logical expression to check the dtb is incorrect in > load_devicetree. > > The problem was indicated by cppcheck. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > Reviewed-by: Hannes Schmelzer <oe5hpm@oevsv.at> > Acked-by: Hannes Schmelzer <oe5hpm@oevsv.at> > --- > I do not have a board for testing. > Please, review carefully. > --- > board/BuR/common/common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/board/BuR/common/common.c b/board/BuR/common/common.c > index 876150402c..c0316b9ebd 100644 > --- a/board/BuR/common/common.c > +++ b/board/BuR/common/common.c > @@ -265,7 +265,7 @@ static int load_devicetree(void) > char *dtbname = getenv("dtb"); > char *dtbdev = getenv("dtbdev"); > char *dtppart = getenv("dtbpart"); > - if (!dtbdev || !dtbdev || !dtbname) { > + if (!dtbdev || !dtbpart || !dtbname) { > printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__); > return -1; > } This breaks some boards such as brppt1_mmc, which I agree doesn't make a lot of sense with just the above context, can you please test building 'BuR' via buildman? Thanks!
On Fri, 5 May 2017 12:38:06 -0400 Tom Rini trini@konsulko.com wrote: ... > > char *dtbname = getenv("dtb"); > > char *dtbdev = getenv("dtbdev"); > > char *dtppart = getenv("dtbpart"); > > - if (!dtbdev || !dtbdev || !dtbname) { > > + if (!dtbdev || !dtbpart || !dtbname) { > > printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__); > > return -1; > > } > > This breaks some boards such as brppt1_mmc, which I agree doesn't make a > lot of sense with just the above context, can you please test building > 'BuR' via buildman? Thanks! dtbpart is wrong here, the defined variable is dtppart. -- Anatolij
On Fri, May 05, 2017 at 08:11:54PM +0200, Anatolij Gustschin wrote: > On Fri, 5 May 2017 12:38:06 -0400 > Tom Rini trini@konsulko.com wrote: > ... > > > char *dtbname = getenv("dtb"); > > > char *dtbdev = getenv("dtbdev"); > > > char *dtppart = getenv("dtbpart"); > > > - if (!dtbdev || !dtbdev || !dtbname) { > > > + if (!dtbdev || !dtbpart || !dtbname) { > > > printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__); > > > return -1; > > > } > > > > This breaks some boards such as brppt1_mmc, which I agree doesn't make a > > lot of sense with just the above context, can you please test building > > 'BuR' via buildman? Thanks! > > dtbpart is wrong here, the defined variable is dtppart. Ah-ha! Time to clean off the laptop screen...
On 05/05/2017 08:14 PM, Tom Rini wrote: > On Fri, May 05, 2017 at 08:11:54PM +0200, Anatolij Gustschin wrote: >> On Fri, 5 May 2017 12:38:06 -0400 >> Tom Rini trini@konsulko.com wrote: >> ... >>>> char *dtbname = getenv("dtb"); >>>> char *dtbdev = getenv("dtbdev"); >>>> char *dtppart = getenv("dtbpart"); >>>> - if (!dtbdev || !dtbdev || !dtbname) { >>>> + if (!dtbdev || !dtbpart || !dtbname) { >>>> printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__); >>>> return -1; >>>> } >>> This breaks some boards such as brppt1_mmc, which I agree doesn't make a >>> lot of sense with just the above context, can you please test building >>> 'BuR' via buildman? Thanks! >> dtbpart is wrong here, the defined variable is dtppart. > Ah-ha! Time to clean off the laptop screen... Yeah, maybe we should also do some cosmetic cleanup on this with an extra patch. This typo is here since the early days ;-) Heinrich, could you do so? cheers, Hannes
diff --git a/board/BuR/common/common.c b/board/BuR/common/common.c index 876150402c..c0316b9ebd 100644 --- a/board/BuR/common/common.c +++ b/board/BuR/common/common.c @@ -265,7 +265,7 @@ static int load_devicetree(void) char *dtbname = getenv("dtb"); char *dtbdev = getenv("dtbdev"); char *dtppart = getenv("dtbpart"); - if (!dtbdev || !dtbdev || !dtbname) { + if (!dtbdev || !dtbpart || !dtbname) { printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__); return -1; }
The logical expression to check the dtb is incorrect in load_devicetree. The problem was indicated by cppcheck. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- I do not have a board for testing. Please, review carefully. --- board/BuR/common/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)