Change in simtrace2[master]: firmware: Enable -Wformat and resolve all related compiler warnings

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Sun Aug 26 08:25:38 UTC 2018


Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/10608 )

Change subject: firmware: Enable -Wformat and resolve all related compiler warnings
......................................................................

firmware: Enable -Wformat and resolve all related compiler warnings

There have been tons of format-string related bugs in our code which
we never discovered due to disabling -Wformat.  Let's fix that.

Change-Id: I5ec466361bcc526fac1f4897673264ee5af3458b
---
M firmware/Makefile
M firmware/apps/cardem/main.c
M firmware/apps/trace/main.c
M firmware/libboard/common/source/boardver_adc.c
M firmware/libboard/qmod/source/board_qmod.c
M firmware/libcommon/source/card_emu.c
M firmware/libcommon/source/host_communication.c
M firmware/libcommon/source/iso7816_4.c
M firmware/libcommon/source/mode_cardemu.c
M firmware/libcommon/source/pseudo_talloc.c
M firmware/libcommon/source/simtrace_iso7816.c
M firmware/libcommon/source/sniffer.c
M firmware/libcommon/source/stdio.c
13 files changed, 24 insertions(+), 23 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Harald Welte: Looks good to me, approved



diff --git a/firmware/Makefile b/firmware/Makefile
index c9039a3..c0f53f3 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -141,13 +141,12 @@
 INCLUDES += -Isrc_simtrace -Iinclude
 INCLUDES += -Iapps/$(APP)
 
-CFLAGS += -Wall -Wchar-subscripts -Wcomment -Wimplicit-int #-Wformat=2
+CFLAGS += -Wall -Wchar-subscripts -Wcomment -Wimplicit-int -Wformat=2
 CFLAGS += -Werror-implicit-function-declaration -Wmain -Wparentheses
 CFLAGS += -Wsequence-point -Wreturn-type -Wswitch -Wtrigraphs #-Wunused
 CFLAGS += -Wuninitialized -Wunknown-pragmas -Wfloat-equal #-Wundef
 CFLAGS += -Wshadow -Wpointer-arith -Wbad-function-cast -Wwrite-strings
 CFLAGS += -Waggregate-return #-Wsign-compare
-CFLAGS += -Wformat=0
 CFLAGS += -Wmissing-format-attribute -Wno-deprecated-declarations
 CFLAGS += #-Wpacked
 CFLAGS += -Wredundant-decls -Wnested-externs #-Winline -Wlong-long
diff --git a/firmware/apps/cardem/main.c b/firmware/apps/cardem/main.c
index 6cd3439..600e4e9 100644
--- a/firmware/apps/cardem/main.c
+++ b/firmware/apps/cardem/main.c
@@ -158,7 +158,7 @@
 		"=============================================================================\n\r");
 
 #if (TRACE_LEVEL >= TRACE_LEVEL_INFO)
-	TRACE_INFO("Chip ID: 0x%08x (Ext 0x%08x)\n\r", CHIPID->CHIPID_CIDR, CHIPID->CHIPID_EXID);
+	TRACE_INFO("Chip ID: 0x%08lx (Ext 0x%08lx)\n\r", CHIPID->CHIPID_CIDR, CHIPID->CHIPID_EXID);
 	TRACE_INFO("Serial Nr. %08x-%08x-%08x-%08x\n\r",
 		   g_unique_id[0], g_unique_id[1],
 		   g_unique_id[2], g_unique_id[3]);
@@ -173,7 +173,7 @@
 	if (reset_cause < ARRAY_SIZE(reset_causes)) {
 		TRACE_INFO("Reset Cause: %s\n\r", reset_causes[reset_cause]);
 	} else {
-		TRACE_INFO("Reset Cause: 0x%x\n\r", (RSTC->RSTC_SR & RSTC_SR_RSTTYP_Msk) >> RSTC_SR_RSTTYP_Pos);
+		TRACE_INFO("Reset Cause: 0x%lx\n\r", (RSTC->RSTC_SR & RSTC_SR_RSTTYP_Msk) >> RSTC_SR_RSTTYP_Pos);
 	}
 #endif
 
diff --git a/firmware/apps/trace/main.c b/firmware/apps/trace/main.c
index f7eb15d..97455fb 100644
--- a/firmware/apps/trace/main.c
+++ b/firmware/apps/trace/main.c
@@ -165,11 +165,11 @@
 		"SIMtrace2 firmware " GIT_VERSION " (C) 2010-2016 by Harald Welte\n\r"
 		"=============================================================================\n\r");
 
-	TRACE_INFO("Chip ID: 0x%08x (Ext 0x%08x)\n\r", CHIPID->CHIPID_CIDR, CHIPID->CHIPID_EXID);
+	TRACE_INFO("Chip ID: 0x%08lx (Ext 0x%08lx)\n\r", CHIPID->CHIPID_CIDR, CHIPID->CHIPID_EXID);
 	TRACE_INFO("Serial Nr. %08x-%08x-%08x-%08x\n\r",
 		   g_unique_id[0], g_unique_id[1],
 		   g_unique_id[2], g_unique_id[3]);
-	TRACE_INFO("Reset Cause: 0x%x\n\r", (RSTC->RSTC_SR & RSTC_SR_RSTTYP_Msk) >> RSTC_SR_RSTTYP_Pos);
+	TRACE_INFO("Reset Cause: 0x%lx\n\r", (RSTC->RSTC_SR & RSTC_SR_RSTTYP_Msk) >> RSTC_SR_RSTTYP_Pos);
 	TRACE_INFO("USB configuration used: %d\n\r", simtrace_config);
 
 	board_main_top();
diff --git a/firmware/libboard/common/source/boardver_adc.c b/firmware/libboard/common/source/boardver_adc.c
index 51db929..11799cc 100644
--- a/firmware/libboard/common/source/boardver_adc.c
+++ b/firmware/libboard/common/source/boardver_adc.c
@@ -86,7 +86,7 @@
 	/* convert to voltage */
 	sample = ADC->ADC_CDR[2];
 	uv = adc2uv(sample);
-	TRACE_INFO("VERSION_DET ADC=%u => %u uV\r\n", sample, uv);
+	TRACE_INFO("VERSION_DET ADC=%u => %lu uV\r\n", sample, uv);
 
 	/* FIXME: convert to board version based on thresholds */
 
diff --git a/firmware/libboard/qmod/source/board_qmod.c b/firmware/libboard/qmod/source/board_qmod.c
index bdc08f9..f04bc12 100644
--- a/firmware/libboard/qmod/source/board_qmod.c
+++ b/firmware/libboard/qmod/source/board_qmod.c
@@ -163,13 +163,13 @@
 		UART_GetIntegerMinMax(&addr, 0, 255);
 		printf("Please enter EEPROM value:\n\r");
 		UART_GetIntegerMinMax(&val, 0, 255);
-		printf("Writing value 0x%02x to EEPROM offset 0x%02x\n\r", val, addr);
+		printf("Writing value 0x%02lx to EEPROM offset 0x%02lx\n\r", val, addr);
 		eeprom_write_byte(0x50, addr, val);
 		break;
 	case 'r':
 		printf("Please enter EEPROM offset:\n\r");
 		UART_GetIntegerMinMax(&addr, 0, 255);
-		printf("EEPROM[0x%02x] = 0x%02x\n\r", addr, eeprom_read_byte(0x50, addr));
+		printf("EEPROM[0x%02lx] = 0x%02x\n\r", addr, eeprom_read_byte(0x50, addr));
 		break;
 	default:
 		printf("Unknown command '%c'\n\r", ch);
diff --git a/firmware/libcommon/source/card_emu.c b/firmware/libcommon/source/card_emu.c
index af90351..75910c1 100644
--- a/firmware/libcommon/source/card_emu.c
+++ b/firmware/libcommon/source/card_emu.c
@@ -112,7 +112,7 @@
 #define	_P3	4
 
 struct card_handle {
-	uint32_t num;
+	unsigned int num;
 
 	enum iso7816_3_card_state state;
 
diff --git a/firmware/libcommon/source/host_communication.c b/firmware/libcommon/source/host_communication.c
index 783c976..ea573bb 100644
--- a/firmware/libcommon/source/host_communication.c
+++ b/firmware/libcommon/source/host_communication.c
@@ -150,7 +150,7 @@
 	rc = USBD_Read(ep, msg->head, msgb_tailroom(msg),
 			(TransferCallback) &usb_read_cb, msg);
 	if (rc != USBD_STATUS_SUCCESS) {
-		TRACE_ERROR("%s error %s\n", __func__, rc);
+		TRACE_ERROR("%s error %d\n", __func__, rc);
 		usb_buf_free(msg);
 		bep->in_progress = 0;
 	}
diff --git a/firmware/libcommon/source/iso7816_4.c b/firmware/libcommon/source/iso7816_4.c
index ac15306..34b84a9 100644
--- a/firmware/libcommon/source/iso7816_4.c
+++ b/firmware/libcommon/source/iso7816_4.c
@@ -140,8 +140,8 @@
 	while((us_base->US_CSR & (US_CSR_TXRDY)) == 0)  {
 		i++;
 		if (!(i%1000000)) {
-			printf("s: %x ", us_base->US_CSR);
-			printf("s: %x\r\n", us_base->US_RHR & 0xFF);
+			printf("s: %lx ", us_base->US_CSR);
+			printf("s: %lx\r\n", us_base->US_RHR & 0xFF);
 			us_base->US_CR = US_CR_RSTTX;
 			us_base->US_CR = US_CR_RSTRX;
 	  }
diff --git a/firmware/libcommon/source/mode_cardemu.c b/firmware/libcommon/source/mode_cardemu.c
index 76b3a01..98818e1 100644
--- a/firmware/libcommon/source/mode_cardemu.c
+++ b/firmware/libcommon/source/mode_cardemu.c
@@ -50,7 +50,7 @@
 #endif
 
 struct cardem_inst {
-	uint32_t num;
+	unsigned int num;
 	struct card_handle *ch;
 	struct llist_head usb_out_queue;
 	struct ringbuf rb;
@@ -117,7 +117,7 @@
 	/* wait until last char has been fully transmitted */
 	while ((usart->US_CSR & (US_CSR_TXEMPTY)) == 0) {
 		if (!(i%1000000)) {
-			TRACE_ERROR("s: %x \r\n", usart->US_CSR);
+			TRACE_ERROR("s: %lx \r\n", usart->US_CSR);
 		}
 		i++;
 	}
@@ -174,7 +174,7 @@
 	int i = 1;
 	while ((usart->US_CSR & (US_CSR_TXRDY)) == 0) {
 		if (!(i%1000000)) {
-			TRACE_ERROR("%u: s: %x %02X\r\n",
+			TRACE_ERROR("%u: s: %lx %02lX\r\n",
 				uart_chan, usart->US_CSR,
 				usart->US_RHR & 0xFF);
 			usart->US_CR = US_CR_RSTTX;
@@ -213,7 +213,7 @@
 	if (csr & (US_CSR_OVRE|US_CSR_FRAME|US_CSR_PARE|
 		   US_CSR_TIMEOUT|US_CSR_NACK|(1<<10))) {
 		usart->US_CR = US_CR_RSTSTA | US_CR_RSTIT | US_CR_RSTNACK;
-		TRACE_ERROR("%u e 0x%x st: 0x%x\n", ci->num, byte, csr);
+		TRACE_ERROR("%u e 0x%x st: 0x%lx\n", ci->num, byte, csr);
 	}
 }
 
diff --git a/firmware/libcommon/source/pseudo_talloc.c b/firmware/libcommon/source/pseudo_talloc.c
index 564c3ef..862fffd 100644
--- a/firmware/libcommon/source/pseudo_talloc.c
+++ b/firmware/libcommon/source/pseudo_talloc.c
@@ -62,7 +62,7 @@
 	for (i = 0; i < ARRAY_SIZE(msgb_inuse); i++) {
 		if (ptr == msgb_data[i]) {
 			if (!msgb_inuse[i]) {
-				TRACE_ERROR("%s: double_free by \r\n", __func__, location);
+				TRACE_ERROR("%s: double_free by %s\r\n", __func__, location);
 			} else {
 				msgb_inuse[i] = 0;
 			}
diff --git a/firmware/libcommon/source/simtrace_iso7816.c b/firmware/libcommon/source/simtrace_iso7816.c
index 6c0d363..889ca2d 100644
--- a/firmware/libcommon/source/simtrace_iso7816.c
+++ b/firmware/libcommon/source/simtrace_iso7816.c
@@ -58,7 +58,7 @@
 {
 	int ret;
 	// FIXME: no printfs in ISRs?
-	printf("+++ Int!! %x\n\r", pinPhoneRST.pio->PIO_ISR);
+	printf("+++ Int!! %lx\n\r", pinPhoneRST.pio->PIO_ISR);
 	if (((pinPhoneRST.pio->PIO_ISR & pinPhoneRST.mask) != 0)) {
 		if (PIO_Get(&pinPhoneRST) == 0) {
 			printf(" 0 ");
diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c
index 3cd0b89..33c16d3 100644
--- a/firmware/libcommon/source/sniffer.c
+++ b/firmware/libcommon/source/sniffer.c
@@ -215,7 +215,7 @@
 		wt_d = d;
 	}
 	wt = wt_wi*960UL*wt_d;
-	TRACE_INFO("WT updated to %u\n\r", wt);
+	TRACE_INFO("WT updated to %lu\n\r", wt);
 }
 
 /*! Allocate USB buffer and push + initialize simtrace_msg_hdr
@@ -325,7 +325,7 @@
 	uint32_t i;
 	for (i = 0; i < nb_flags; i++) {
 		if (flags & flag_meanings[i].value) {
-			printf(flag_meanings[i].str);
+			printf("%s", flag_meanings[i].str);
 			flags &= ~flag_meanings[i].value;
 			if (flags) {
 				printf(", ");
diff --git a/firmware/libcommon/source/stdio.c b/firmware/libcommon/source/stdio.c
index 06ad611..04d73f0 100644
--- a/firmware/libcommon/source/stdio.c
+++ b/firmware/libcommon/source/stdio.c
@@ -62,8 +62,10 @@
 //------------------------------------------------------------------------------
 //
 FILE* const stdin = NULL;
-FILE* const stdout = NULL;
-FILE* const stderr = NULL;
+/* If we use NULL here, we get compiler warnings of calling stdio functions with
+ * NULL values.  Our fputs() implementation ignores the value of those pointers anyway */
+FILE* const stdout = (FILE *) 0x1;
+FILE* const stderr = (FILE *) 0x2;
 
 
 //------------------------------------------------------------------------------

-- 
To view, visit https://gerrit.osmocom.org/10608
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: simtrace2
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5ec466361bcc526fac1f4897673264ee5af3458b
Gerrit-Change-Number: 10608
Gerrit-PatchSet: 3
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180826/7255f048/attachment.htm>


More information about the gerrit-log mailing list