cppcheck cleanup

The following patches clean up a number of errors and warnings detected by running the "cppcheck" [1] tool over the U-Boot source tree. This discovers a number of issues that escape GCC, for example because they are inside #ifdef branches which are never compiled for any of the tested configurations.

This is just an initial patch series; some other error reports have been sent to the respective code maintainers, so more patches should follow soon.

The command used for these tests was:

	$ cd u-boot
	$ cppcheck --force --quiet --inline-suppr .

[1] http://cppcheck.sourceforge.net/

Wolfgang Denk (9):
cppcheck cleanup: fix nullPointer errors board/cogent/lcd.c: fix syntax error drivers/usb/host/isp116x-hcd.c: fix syntax error drivers/net/uli526x.c: fix syntax error common/cmd_fitupd.c: restore corrupted file board/matrix_vision/mvblx/sys_eeprom.c: fix buffer overflow board/renesas/ecovec/ecovec.c: fix buffer overflow
ARM: MXS: fix Uninitialized variable error board/esd/common/auto_update.c: fix Uninitialized variable

 arch/arm/cpu/arm926ejs/mxs/mxs.c          |  2 ++
 arch/arm/cpu/arm926ejs/mxs/spl_boot.c     |  2 ++
 arch/arm/cpu/arm926ejs/mxs/timer.c        |  2 ++
 arch/arm/cpu/armv7/zynq/ddrc.c            |  1 +
 arch/blackfin/cpu/initcode.c              |  1 +
 arch/powerpc/cpu/mpc85xx/cpu_init_early.c |  5 ++++-
 arch/sh/lib/zimageboot.c                  |  1 +
 board/cogent/lcd.c                        |  2 +-
 board/esd/common/auto_update.c            |  3 +--
 board/esd/pci405/cmd_pci405.c             |  4 +++-
 board/keymile/common/common.c             |  1 +
 board/matrix_vision/mvblx/sys_eeprom.c    |  2 +-
 board/renesas/ecovec/ecovec.c             |  2 +-
 board/scb9328/flash.c                     |  2 ++
 common/cmd_fitupd.c                       | 14 +++++---------
 drivers/net/uli526x.c                     |  2 +-
 drivers/usb/host/isp116x-hcd.c            |  2 +-
 17 files changed, 30 insertions(+), 18 deletions(-)

1.8.3.1

There are a number of places where U-Boot intentionally and legally accesses physical address 0x0000, for example when installing exception vectors on systems where these are located in low memory.

Add "cppcheck-suppress nullPointer" comments to silence cppcheck where this is intentional and legal.

---
 arch/arm/cpu/arm926ejs/mxs/mxs.c          | 2 ++
 arch/arm/cpu/arm926ejs/mxs/spl_boot.c     | 2 ++
 arch/arm/cpu/armv7/zynq/ddrc.c            | 1 +
 arch/blackfin/cpu/initcode.c              | 1 +
 arch/powerpc/cpu/mpc85xx/cpu_init_early.c | 5 ++++-
 arch/sh/lib/zimageboot.c                  | 1 +
 board/esd/pci405/cmd_pci405.c             | 4 +++-
 board/keymile/common/common.c             | 1 +
 board/scb9328/flash.c                     | 2 ++
 9 files changed, 17 insertions(+), 2 deletions(-)

index 365542f..ef130ae 100644
@@ -83,7 +83,9 @@ void mx28_fixup_vt(uint32_t start_addr)
 	int i;
 
 	for (i = 0; i < 8; i++) {
+		/* cppcheck-suppress nullPointer */
 		vt[i] = ldr_pc;
+		/* cppcheck-suppress nullPointer */
 		vt[i + 8] = start_addr + (4 * i);
 	}
 }

index d3e1369..d29b9aa 100644
@@ -118,6 +118,8 @@ static void mxs_spl_fixup_vectors(void)
 	 * fine.
 	 */
 	extern uint32_t _start;
+
+	/* cppcheck-suppress nullPointer */
 	memcpy(0x0, &_start, 0x60);
 }
 

index 1ea086d..d74f8db 100644
@@ -40,6 +40,7 @@ void zynq_ddrc_init(void)
 		 * first stage bootloader. To get ECC to work all memory has
 		 * been initialized by writing any value.
 		 */
+		/* cppcheck-suppress nullPointer */
 		memset((void *)0, 0, 1 * 1024 * 1024);
 	} else {
 		puts("ECC disabled ");

index 2e640af..fde54ea 100644
@@ -955,6 +955,7 @@ check_hibernation(ADI_BOOT_DATA *bs, u16 vr_ctl, bool put_into_srfs)
 		uint32_t *hibernate_magic = 0;
 
 		SSYNC();
+		/* cppcheck-suppress nullPointer */
 		if (hibernate_magic[0] == 0xDEADBEEF) {
 			serial_putc('c');
 			bfin_write_EVT15(hibernate_magic[1]);

index 47b712d..072387a 100644
@@ -161,9 +161,12 @@ void cpu_init_early_f(void *fdt)
 	setup_ifc_sram = (void *)SRAM_BASE_ADDR;
 	dst = (u32 *) SRAM_BASE_ADDR;
 	src = (u32 *) setup_ifc;
-	for (i = 0; i < 1024; i++)
+	for (i = 0; i < 1024; i++) {
+		/* cppcheck-suppress nullPointer */
 		*dst++ = *src++;
+	}
 
+	/* cppcheck-suppress nullPointer */
 	setup_ifc_sram();
 
 	/* CLEANUP */

index 86d3998..3fea5f5 100644
@@ -45,6 +45,7 @@ int do_sh_zimageboot (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	bootargs = getenv("bootargs");
 
 	/* Clear zero page */
+	/* cppcheck-suppress nullPointer */
 	memset(param, 0, 0x1000);
 
 	/* Set commandline */

index 55c20d0..29c688a 100644
@@ -23,7 +23,7 @@
  */
 int do_loadpci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	unsigned int *ptr = 0;
+	unsigned int *ptr;
 	int count = 0;
 	int count2 = 0;
 	int i;

@@ -35,12 +35,14 @@ int do_loadpci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	 * Mark sync address
 	 */
 	ptr = 0;
+	/* cppcheck-suppress nullPointer */
 	*ptr = 0xffffffff;
 	puts("\nWaiting for image from pci host -");
 
 	/*
 	 * Wait for host to write the start address
 	 */
+	/* cppcheck-suppress nullPointer */
 	while (*ptr == 0xffffffff) {
 		count++;
 		if (!(count % 100)) {

index 2ddb3da..b9aff1a 100644
@@ -360,6 +360,7 @@ static int do_checktestboot(cmd_tbl_t *cmdtp, int flag, int argc,
 	testboot = (testpin != 0) && (s);
 	if (verbose) {
 		printf("testpin   = %d\n", testpin);
+		/* cppcheck-suppress nullPointer */
 		printf("test_bank = %s\n", s ? s : "not set");
 		printf("boot test app : %s\n", (testboot) ? "yes" : "no");
 	}

index e3a582f..73bfa00 100644
@@ -72,8 +72,10 @@ static FLASH_BUS_RET flash_status_reg (void)
 
 	FLASH_BUS *addr = (FLASH_BUS *) 0;
 
+	/* cppcheck-suppress nullPointer */
 	*addr = FLASH_CMD (CFI_INTEL_CMD_READ_STATUS_REGISTER);
 
+	/* cppcheck-suppress nullPointer */
 	return *addr;
 }
 

1.8.3.1

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141110/a00db655/attachment.pgp>

Fix error detected by cppcheck:

[board/cogent/lcd.c:237]: (error) Invalid number of character (() when these macros are defined:
'CONFIG_SHOW_ACTIVITY;CONFIG_STATUS_LED'.

---
 board/cogent/lcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 8e90f98..05ffc4d 100644
@@ -234,7 +234,7 @@ lcd_heartbeat(void)
 void board_show_activity (ulong timestamp)
 {
 #ifdef CONFIG_STATUS_LED
-	if ((timestamp % (CONFIG_SYS_HZ / 2) == 0)
+	if ((timestamp % (CONFIG_SYS_HZ / 2)) == 0)
 		lcd_heartbeat ();
 #endif
 }

1.8.3.1

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141110/012fc02b/attachment.pgp>

Fix error detected by cppcheck:

[drivers/usb/host/isp116x-hcd.c:1282]: (error) Invalid number of character (() when these macros are defined: ''. [drivers/usb/host/isp116x-hcd.c:1282]: (error) Invalid number of character (() when these macros are defined: 'ISP116X_HCD_OC_ENABLE'. [drivers/usb/host/isp116x-hcd.c:1282]: (error) Invalid number of character (() when these macros are defined:
'ISP116X_HCD_REMOTE_WAKEUP_ENABLE'. [drivers/usb/host/isp116x-hcd.c:1282]: (error) Invalid number of character (() when these macros are defined: 'ISP116X_HCD_SEL15kRES'. [drivers/usb/host/isp116x-hcd.c:1282]: (error) Invalid number of character (() when these macros are defined:
'ISP116X_HCD_USE_EXTRA_DELAY'. [drivers/usb/host/isp116x-hcd.c:1282]: (error) Invalid number of character (() when these macros are defined: 'ISP116X_HCD_USE_UDELAY'. [drivers/usb/host/isp116x-hcd.c:1282]: (error) Invalid number of character (() when these macros are defined: 'TRACE'. [drivers/usb/host/isp116x-hcd.c:1282]: (error) Invalid number of character (() when these macros are defined: 'TRACE;VERBOSE'. [drivers/usb/host/isp116x-hcd.c:1282]: (error) Invalid number of character (() when these macros are defined: 'VERBOSE'.

---
 drivers/usb/host/isp116x-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 46e4cee..35d4e9f 100644
@@ -1279,7 +1279,7 @@ int isp116x_check_id(struct isp116x *isp116x)
 	return 0;
 }
 
-int usb_lowlevel_init(int index, enum usb_init_type init, void **controller))
+int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
 {
 	struct isp116x *isp116x = &isp116x_dev;
 

1.8.3.1

A brief grep:
marex at bfu:u-boot$ git grep USB_ISP116X_HCD drivers/usb/host/Makefile:obj-$(CONFIG_USB_ISP116X_HCD) += isp116x-hcd.o include/usb.h: defined(CONFIG_USB_SL811HS) || defined(CONFIG_USB_ISP116X_HCD) || \
marex at bfu:u-boot$

gives me a better idea on how to resolve this bug ... scrap this code altogether. Looks like the code was unused forever. Can you prepare a patch for that please ?

Best regards,
Marek Vasut

Fix error detected by cppcheck:

[drivers/net/uli526x.c:551]: (error) printf format string requires 3 parameters but only 2 are given.

---
 drivers/net/uli526x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 538f11e..9526faa 100644
@@ -548,7 +548,7 @@ static int uli526x_rx_packet(struct eth_device *dev)
 
 	rdes0 = le32_to_cpu(rxptr->rdes0);
 #ifdef RX_DEBUG
-	printf("%s(): rxptr->rdes0=%x:%x\n", __FUNCTION__, rxptr->rdes0);
+	printf("%s(): rxptr->rdes0=%x\n", __FUNCTION__, rxptr->rdes0);
 #endif
 	if (!(rdes0 & 0x80000000)) {	/* packet owner check */
 		if ((rdes0 & 0x300) != 0x300) {

1.8.3.1

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141110/79920734/attachment.pgp>

This file got corrupted by the automatic editin of commit 1a45966 "Add GPL-2.0+ SPDX-License-Identifier to source files"; restore the opiginal content and manually insert the SPDX ID.

The bug was detected by running cppcheck, which reported:
[common/cmd_fitupd.c:8]: (error) Invalid number of character ({) when these macros are defined: 'CONFIG_UPDATE_TFTP'.

---
 common/cmd_fitupd.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

index e811473..b045974 100644
@@ -1,12 +1,8 @@
-de <net.h>
-
-#if !defined(CONFIG_UPDATE_TFTP)
-#error "CONFIG_UPDATE_TFTP required"
-#endif
-
-static int do_fitupd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
-	ulong addr = 0Un the root directory of the source tree for details.
+/*
+ * (C) Copyright 2011
+ * Andreas Pretzsch, carpe noctem engineering, apr at cn-eng.de
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
  */
 
 #include <common.h>

1.8.3.1

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141110/64615e4c/attachment.pgp>

Fix error detected by cppcheck:

[board/matrix_vision/mvblx/sys_eeprom.c:353]: (error) Buffer is accessed out of bounds.

---
 board/matrix_vision/mvblx/sys_eeprom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 1a2ac8d..db42987 100644
@@ -348,7 +348,7 @@ int mac_read_from_eeprom(void)
 
 	if (memcmp(&e.mac, "\0\0\0\0\0\0", 6) &&
 		memcmp(&e.mac, "\xFF\xFF\xFF\xFF\xFF\xFF", 6)) {
-		char ethaddr[9];
+		char ethaddr[18];
 
 		sprintf(ethaddr, "%02X:%02X:%02X:%02X:%02X:%02X",
 			e.mac[0],

1.8.3.1

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141110/2e65fb20/attachment.pgp>

Fix error detected by cppcheck:

[board/renesas/ecovec/ecovec.c:66]: (error) Buffer is accessed out of bounds.

---
 board/renesas/ecovec/ecovec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 2804d91..d862d99 100644
@@ -41,7 +41,7 @@ static void debug_led(u8 led)
 int board_late_init(void)
 {
 	u8 mac[6];
-	char env_mac[17];
+	char env_mac[18];
 
 	udelay(1000);
 

1.8.3.1

2014-11-06 22:03 GMT+09:00 Wolfgang Denk <[email protected]>:

Applied, thanks.

Best regards,
Nobuhiro

Nobuhiro Iwamatsu
   iwamatsu at {nigauri.org / debian.org}
   GPG ID: 40AD1FA6

cppcheck reports:

[arch/arm/cpu/arm926ejs/mxs/timer.c:96]: (error) Uninitialized
variable: now

---
 arch/arm/cpu/arm926ejs/mxs/timer.c | 2 ++
 1 file changed, 2 insertions(+)

index 99d3fb8..f2e7225 100644
@@ -91,6 +91,8 @@ unsigned long long get_ticks(void)
 		TIMROT_RUNNING_COUNTn_RUNNING_COUNT_OFFSET;
 #elif defined(CONFIG_MX28)
 	now = readl(&timrot_regs->hw_timrot_running_count0);
+#else
+#error "Don't know how to read timrot_regs"
 #endif
 
 	if (lastdec >= now) {

1.8.3.1

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141110/af515f8d/attachment.pgp>

cppcheck reports:

[board/esd/common/auto_update.c:458]: (error) Uninitialized variable: cnt

The variable is not really used anywhere, so remove it.

---
 board/esd/common/auto_update.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index 85c3567..b168074 100644
@@ -377,7 +377,7 @@ int do_auto_update(void)
 {
 	block_dev_desc_t *stor_dev = NULL;
 	long sz;
-	int i, res, cnt, old_ctrlc;
+	int i, res, old_ctrlc;
 	char buffer[32];
 	char str[80];
 	int n;

@@ -455,7 +455,6 @@ int do_auto_update(void)
 				clear_ctrlc ();
 				break;
 			}
-			cnt++;
 		} while (res < 0);
 	}
 

1.8.3.1

Applied to u-boot/master, thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141110/310024f0/attachment.pgp>