[U-Boot] [PATCH] board/BuR/common: incorrect check of dtb

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; }

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

The logical expression to check the dtb is incorrect in load_devicetree.
The problem was indicated by cppcheck.
The inconsistent variable name dtppart is changed to dtbpart.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2: fix syntax error due to incorrect spelling of variable
Building was checked with buildman
$ buildman -k brxre1 boards.cfg is up to date. Nothing to do. Building current source for 1 boards (1 thread, 4 jobs per thread) 1 0 0 /1 brxre1 $ buildman -k brppt1 boards.cfg is up to date. Nothing to do. Building current source for 3 boards (3 threads, 2 jobs per thread) 3 0 0 /3 0:00:45 : brppt1_nand
I do not have a BuR board available for actual testing.
v1: Original patch https://patchwork.ozlabs.org/patch/758237/ --- board/BuR/common/common.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/board/BuR/common/common.c b/board/BuR/common/common.c index 876150402c..5cc82c9473 100644 --- a/board/BuR/common/common.c +++ b/board/BuR/common/common.c @@ -264,13 +264,13 @@ static int load_devicetree(void) #else char *dtbname = getenv("dtb"); char *dtbdev = getenv("dtbdev"); - char *dtppart = getenv("dtbpart"); - if (!dtbdev || !dtbdev || !dtbname) { + char *dtbpart = getenv("dtbpart"); + if (!dtbdev || !dtbpart || !dtbname) { printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__); return -1; }
- if (fs_set_blk_dev(dtbdev, dtppart, FS_TYPE_EXT)) { + if (fs_set_blk_dev(dtbdev, dtbpart, FS_TYPE_EXT)) { puts("load_devicetree: set_blk_dev failed.\n"); return -1; }

The logical expression to check the dtb is incorrect in load_devicetree.
The problem was indicated by cppcheck.
The inconsistent variable name dtppart is changed to dtbpart.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v2: fix syntax error due to incorrect spelling of variable
Building was checked with buildman
$ buildman -k brxre1 boards.cfg is up to date. Nothing to do. Building current source for 1 boards (1 thread, 4 jobs per thread) 1 0 0 /1 brxre1 $ buildman -k brppt1 boards.cfg is up to date. Nothing to do. Building current source for 3 boards (3 threads, 2 jobs per thread) 3 0 0 /3 0:00:45 : brppt1_nand
I do not have a BuR board available for actual testing.
v1: Original patch https://patchwork.ozlabs.org/patch/758237/
board/BuR/common/common.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/board/BuR/common/common.c b/board/BuR/common/common.c index 876150402c..5cc82c9473 100644 --- a/board/BuR/common/common.c +++ b/board/BuR/common/common.c @@ -264,13 +264,13 @@ static int load_devicetree(void) #else char *dtbname = getenv("dtb"); char *dtbdev = getenv("dtbdev");
- char *dtppart = getenv("dtbpart");
- if (!dtbdev || !dtbdev || !dtbname) {
- char *dtbpart = getenv("dtbpart");
- if (!dtbdev || !dtbpart || !dtbname) { printf("%s: <dtbdev>/<dtbpart>/<dtb> missing.\n", __func__); return -1; }
- if (fs_set_blk_dev(dtbdev, dtppart, FS_TYPE_EXT)) {
- if (fs_set_blk_dev(dtbdev, dtbpart, FS_TYPE_EXT)) { puts("load_devicetree: set_blk_dev failed.\n"); return -1; }
Reviewed-by: Hannes Schmelzer oe5hpm@oevsv.at Acked-by: Hannes Schmelzer oe5hpm@oevsv.at

On Fri, May 05, 2017 at 08:57:32PM +0200, xypron.glpk@gmx.de wrote:
The logical expression to check the dtb is incorrect in load_devicetree.
The problem was indicated by cppcheck.
The inconsistent variable name dtppart is changed to dtbpart.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Hannes Schmelzer oe5hpm@oevsv.at Acked-by: Hannes Schmelzer oe5hpm@oevsv.at
Applied to u-boot/master, thanks!
participants (4)
-
Anatolij Gustschin
-
Hannes Schmelzer
-
Heinrich Schuchardt
-
Tom Rini