<p>Harald Welte <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/10608">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">firmware: Enable -Wformat and resolve all related compiler warnings<br><br>There have been tons of format-string related bugs in our code which<br>we never discovered due to disabling -Wformat. Let's fix that.<br><br>Change-Id: I5ec466361bcc526fac1f4897673264ee5af3458b<br>---<br>M firmware/Makefile<br>M firmware/apps/cardem/main.c<br>M firmware/apps/trace/main.c<br>M firmware/libboard/common/source/boardver_adc.c<br>M firmware/libboard/qmod/source/board_qmod.c<br>M firmware/libcommon/source/card_emu.c<br>M firmware/libcommon/source/host_communication.c<br>M firmware/libcommon/source/iso7816_4.c<br>M firmware/libcommon/source/mode_cardemu.c<br>M firmware/libcommon/source/pseudo_talloc.c<br>M firmware/libcommon/source/simtrace_iso7816.c<br>M firmware/libcommon/source/sniffer.c<br>M firmware/libcommon/source/stdio.c<br>13 files changed, 24 insertions(+), 23 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/firmware/Makefile b/firmware/Makefile</span><br><span>index c9039a3..c0f53f3 100644</span><br><span>--- a/firmware/Makefile</span><br><span>+++ b/firmware/Makefile</span><br><span>@@ -141,13 +141,12 @@</span><br><span> INCLUDES += -Isrc_simtrace -Iinclude</span><br><span> INCLUDES += -Iapps/$(APP)</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-CFLAGS += -Wall -Wchar-subscripts -Wcomment -Wimplicit-int #-Wformat=2</span><br><span style="color: hsl(120, 100%, 40%);">+CFLAGS += -Wall -Wchar-subscripts -Wcomment -Wimplicit-int -Wformat=2</span><br><span> CFLAGS += -Werror-implicit-function-declaration -Wmain -Wparentheses</span><br><span> CFLAGS += -Wsequence-point -Wreturn-type -Wswitch -Wtrigraphs #-Wunused</span><br><span> CFLAGS += -Wuninitialized -Wunknown-pragmas -Wfloat-equal #-Wundef</span><br><span> CFLAGS += -Wshadow -Wpointer-arith -Wbad-function-cast -Wwrite-strings</span><br><span> CFLAGS += -Waggregate-return #-Wsign-compare</span><br><span style="color: hsl(0, 100%, 40%);">-CFLAGS += -Wformat=0</span><br><span> CFLAGS += -Wmissing-format-attribute -Wno-deprecated-declarations</span><br><span> CFLAGS += #-Wpacked</span><br><span> CFLAGS += -Wredundant-decls -Wnested-externs #-Winline -Wlong-long</span><br><span>diff --git a/firmware/apps/cardem/main.c b/firmware/apps/cardem/main.c</span><br><span>index 6cd3439..600e4e9 100644</span><br><span>--- a/firmware/apps/cardem/main.c</span><br><span>+++ b/firmware/apps/cardem/main.c</span><br><span>@@ -158,7 +158,7 @@</span><br><span> "=============================================================================\n\r");</span><br><span> </span><br><span> #if (TRACE_LEVEL >= TRACE_LEVEL_INFO)</span><br><span style="color: hsl(0, 100%, 40%);">- TRACE_INFO("Chip ID: 0x%08x (Ext 0x%08x)\n\r", CHIPID->CHIPID_CIDR, CHIPID->CHIPID_EXID);</span><br><span style="color: hsl(120, 100%, 40%);">+ TRACE_INFO("Chip ID: 0x%08lx (Ext 0x%08lx)\n\r", CHIPID->CHIPID_CIDR, CHIPID->CHIPID_EXID);</span><br><span> TRACE_INFO("Serial Nr. %08x-%08x-%08x-%08x\n\r",</span><br><span> g_unique_id[0], g_unique_id[1],</span><br><span> g_unique_id[2], g_unique_id[3]);</span><br><span>@@ -173,7 +173,7 @@</span><br><span> if (reset_cause < ARRAY_SIZE(reset_causes)) {</span><br><span> TRACE_INFO("Reset Cause: %s\n\r", reset_causes[reset_cause]);</span><br><span> } else {</span><br><span style="color: hsl(0, 100%, 40%);">- TRACE_INFO("Reset Cause: 0x%x\n\r", (RSTC->RSTC_SR & RSTC_SR_RSTTYP_Msk) >> RSTC_SR_RSTTYP_Pos);</span><br><span style="color: hsl(120, 100%, 40%);">+ TRACE_INFO("Reset Cause: 0x%lx\n\r", (RSTC->RSTC_SR & RSTC_SR_RSTTYP_Msk) >> RSTC_SR_RSTTYP_Pos);</span><br><span> }</span><br><span> #endif</span><br><span> </span><br><span>diff --git a/firmware/apps/trace/main.c b/firmware/apps/trace/main.c</span><br><span>index f7eb15d..97455fb 100644</span><br><span>--- a/firmware/apps/trace/main.c</span><br><span>+++ b/firmware/apps/trace/main.c</span><br><span>@@ -165,11 +165,11 @@</span><br><span> "SIMtrace2 firmware " GIT_VERSION " (C) 2010-2016 by Harald Welte\n\r"</span><br><span> "=============================================================================\n\r");</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- TRACE_INFO("Chip ID: 0x%08x (Ext 0x%08x)\n\r", CHIPID->CHIPID_CIDR, CHIPID->CHIPID_EXID);</span><br><span style="color: hsl(120, 100%, 40%);">+ TRACE_INFO("Chip ID: 0x%08lx (Ext 0x%08lx)\n\r", CHIPID->CHIPID_CIDR, CHIPID->CHIPID_EXID);</span><br><span> TRACE_INFO("Serial Nr. %08x-%08x-%08x-%08x\n\r",</span><br><span> g_unique_id[0], g_unique_id[1],</span><br><span> g_unique_id[2], g_unique_id[3]);</span><br><span style="color: hsl(0, 100%, 40%);">- TRACE_INFO("Reset Cause: 0x%x\n\r", (RSTC->RSTC_SR & RSTC_SR_RSTTYP_Msk) >> RSTC_SR_RSTTYP_Pos);</span><br><span style="color: hsl(120, 100%, 40%);">+ TRACE_INFO("Reset Cause: 0x%lx\n\r", (RSTC->RSTC_SR & RSTC_SR_RSTTYP_Msk) >> RSTC_SR_RSTTYP_Pos);</span><br><span> TRACE_INFO("USB configuration used: %d\n\r", simtrace_config);</span><br><span> </span><br><span> board_main_top();</span><br><span>diff --git a/firmware/libboard/common/source/boardver_adc.c b/firmware/libboard/common/source/boardver_adc.c</span><br><span>index 51db929..11799cc 100644</span><br><span>--- a/firmware/libboard/common/source/boardver_adc.c</span><br><span>+++ b/firmware/libboard/common/source/boardver_adc.c</span><br><span>@@ -86,7 +86,7 @@</span><br><span> /* convert to voltage */</span><br><span> sample = ADC->ADC_CDR[2];</span><br><span> uv = adc2uv(sample);</span><br><span style="color: hsl(0, 100%, 40%);">- TRACE_INFO("VERSION_DET ADC=%u => %u uV\r\n", sample, uv);</span><br><span style="color: hsl(120, 100%, 40%);">+ TRACE_INFO("VERSION_DET ADC=%u => %lu uV\r\n", sample, uv);</span><br><span> </span><br><span> /* FIXME: convert to board version based on thresholds */</span><br><span> </span><br><span>diff --git a/firmware/libboard/qmod/source/board_qmod.c b/firmware/libboard/qmod/source/board_qmod.c</span><br><span>index bdc08f9..f04bc12 100644</span><br><span>--- a/firmware/libboard/qmod/source/board_qmod.c</span><br><span>+++ b/firmware/libboard/qmod/source/board_qmod.c</span><br><span>@@ -163,13 +163,13 @@</span><br><span> UART_GetIntegerMinMax(&addr, 0, 255);</span><br><span> printf("Please enter EEPROM value:\n\r");</span><br><span> UART_GetIntegerMinMax(&val, 0, 255);</span><br><span style="color: hsl(0, 100%, 40%);">- printf("Writing value 0x%02x to EEPROM offset 0x%02x\n\r", val, addr);</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("Writing value 0x%02lx to EEPROM offset 0x%02lx\n\r", val, addr);</span><br><span> eeprom_write_byte(0x50, addr, val);</span><br><span> break;</span><br><span> case 'r':</span><br><span> printf("Please enter EEPROM offset:\n\r");</span><br><span> UART_GetIntegerMinMax(&addr, 0, 255);</span><br><span style="color: hsl(0, 100%, 40%);">- printf("EEPROM[0x%02x] = 0x%02x\n\r", addr, eeprom_read_byte(0x50, addr));</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("EEPROM[0x%02lx] = 0x%02x\n\r", addr, eeprom_read_byte(0x50, addr));</span><br><span> break;</span><br><span> default:</span><br><span> printf("Unknown command '%c'\n\r", ch);</span><br><span>diff --git a/firmware/libcommon/source/card_emu.c b/firmware/libcommon/source/card_emu.c</span><br><span>index af90351..75910c1 100644</span><br><span>--- a/firmware/libcommon/source/card_emu.c</span><br><span>+++ b/firmware/libcommon/source/card_emu.c</span><br><span>@@ -112,7 +112,7 @@</span><br><span> #define _P3 4</span><br><span> </span><br><span> struct card_handle {</span><br><span style="color: hsl(0, 100%, 40%);">- uint32_t num;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int num;</span><br><span> </span><br><span> enum iso7816_3_card_state state;</span><br><span> </span><br><span>diff --git a/firmware/libcommon/source/host_communication.c b/firmware/libcommon/source/host_communication.c</span><br><span>index 783c976..ea573bb 100644</span><br><span>--- a/firmware/libcommon/source/host_communication.c</span><br><span>+++ b/firmware/libcommon/source/host_communication.c</span><br><span>@@ -150,7 +150,7 @@</span><br><span> rc = USBD_Read(ep, msg->head, msgb_tailroom(msg),</span><br><span> (TransferCallback) &usb_read_cb, msg);</span><br><span> if (rc != USBD_STATUS_SUCCESS) {</span><br><span style="color: hsl(0, 100%, 40%);">- TRACE_ERROR("%s error %s\n", __func__, rc);</span><br><span style="color: hsl(120, 100%, 40%);">+ TRACE_ERROR("%s error %d\n", __func__, rc);</span><br><span> usb_buf_free(msg);</span><br><span> bep->in_progress = 0;</span><br><span> }</span><br><span>diff --git a/firmware/libcommon/source/iso7816_4.c b/firmware/libcommon/source/iso7816_4.c</span><br><span>index ac15306..34b84a9 100644</span><br><span>--- a/firmware/libcommon/source/iso7816_4.c</span><br><span>+++ b/firmware/libcommon/source/iso7816_4.c</span><br><span>@@ -140,8 +140,8 @@</span><br><span> while((us_base->US_CSR & (US_CSR_TXRDY)) == 0) {</span><br><span> i++;</span><br><span> if (!(i%1000000)) {</span><br><span style="color: hsl(0, 100%, 40%);">- printf("s: %x ", us_base->US_CSR);</span><br><span style="color: hsl(0, 100%, 40%);">- printf("s: %x\r\n", us_base->US_RHR & 0xFF);</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("s: %lx ", us_base->US_CSR);</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("s: %lx\r\n", us_base->US_RHR & 0xFF);</span><br><span> us_base->US_CR = US_CR_RSTTX;</span><br><span> us_base->US_CR = US_CR_RSTRX;</span><br><span> }</span><br><span>diff --git a/firmware/libcommon/source/mode_cardemu.c b/firmware/libcommon/source/mode_cardemu.c</span><br><span>index 76b3a01..98818e1 100644</span><br><span>--- a/firmware/libcommon/source/mode_cardemu.c</span><br><span>+++ b/firmware/libcommon/source/mode_cardemu.c</span><br><span>@@ -50,7 +50,7 @@</span><br><span> #endif</span><br><span> </span><br><span> struct cardem_inst {</span><br><span style="color: hsl(0, 100%, 40%);">- uint32_t num;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int num;</span><br><span> struct card_handle *ch;</span><br><span> struct llist_head usb_out_queue;</span><br><span> struct ringbuf rb;</span><br><span>@@ -117,7 +117,7 @@</span><br><span> /* wait until last char has been fully transmitted */</span><br><span> while ((usart->US_CSR & (US_CSR_TXEMPTY)) == 0) {</span><br><span> if (!(i%1000000)) {</span><br><span style="color: hsl(0, 100%, 40%);">- TRACE_ERROR("s: %x \r\n", usart->US_CSR);</span><br><span style="color: hsl(120, 100%, 40%);">+ TRACE_ERROR("s: %lx \r\n", usart->US_CSR);</span><br><span> }</span><br><span> i++;</span><br><span> }</span><br><span>@@ -174,7 +174,7 @@</span><br><span> int i = 1;</span><br><span> while ((usart->US_CSR & (US_CSR_TXRDY)) == 0) {</span><br><span> if (!(i%1000000)) {</span><br><span style="color: hsl(0, 100%, 40%);">- TRACE_ERROR("%u: s: %x %02X\r\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ TRACE_ERROR("%u: s: %lx %02lX\r\n",</span><br><span> uart_chan, usart->US_CSR,</span><br><span> usart->US_RHR & 0xFF);</span><br><span> usart->US_CR = US_CR_RSTTX;</span><br><span>@@ -213,7 +213,7 @@</span><br><span> if (csr & (US_CSR_OVRE|US_CSR_FRAME|US_CSR_PARE|</span><br><span> US_CSR_TIMEOUT|US_CSR_NACK|(1<<10))) {</span><br><span> usart->US_CR = US_CR_RSTSTA | US_CR_RSTIT | US_CR_RSTNACK;</span><br><span style="color: hsl(0, 100%, 40%);">- TRACE_ERROR("%u e 0x%x st: 0x%x\n", ci->num, byte, csr);</span><br><span style="color: hsl(120, 100%, 40%);">+ TRACE_ERROR("%u e 0x%x st: 0x%lx\n", ci->num, byte, csr);</span><br><span> }</span><br><span> }</span><br><span> </span><br><span>diff --git a/firmware/libcommon/source/pseudo_talloc.c b/firmware/libcommon/source/pseudo_talloc.c</span><br><span>index 564c3ef..862fffd 100644</span><br><span>--- a/firmware/libcommon/source/pseudo_talloc.c</span><br><span>+++ b/firmware/libcommon/source/pseudo_talloc.c</span><br><span>@@ -62,7 +62,7 @@</span><br><span> for (i = 0; i < ARRAY_SIZE(msgb_inuse); i++) {</span><br><span> if (ptr == msgb_data[i]) {</span><br><span> if (!msgb_inuse[i]) {</span><br><span style="color: hsl(0, 100%, 40%);">- TRACE_ERROR("%s: double_free by \r\n", __func__, location);</span><br><span style="color: hsl(120, 100%, 40%);">+ TRACE_ERROR("%s: double_free by %s\r\n", __func__, location);</span><br><span> } else {</span><br><span> msgb_inuse[i] = 0;</span><br><span> }</span><br><span>diff --git a/firmware/libcommon/source/simtrace_iso7816.c b/firmware/libcommon/source/simtrace_iso7816.c</span><br><span>index 6c0d363..889ca2d 100644</span><br><span>--- a/firmware/libcommon/source/simtrace_iso7816.c</span><br><span>+++ b/firmware/libcommon/source/simtrace_iso7816.c</span><br><span>@@ -58,7 +58,7 @@</span><br><span> {</span><br><span> int ret;</span><br><span> // FIXME: no printfs in ISRs?</span><br><span style="color: hsl(0, 100%, 40%);">- printf("+++ Int!! %x\n\r", pinPhoneRST.pio->PIO_ISR);</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("+++ Int!! %lx\n\r", pinPhoneRST.pio->PIO_ISR);</span><br><span> if (((pinPhoneRST.pio->PIO_ISR & pinPhoneRST.mask) != 0)) {</span><br><span> if (PIO_Get(&pinPhoneRST) == 0) {</span><br><span> printf(" 0 ");</span><br><span>diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c</span><br><span>index 3cd0b89..33c16d3 100644</span><br><span>--- a/firmware/libcommon/source/sniffer.c</span><br><span>+++ b/firmware/libcommon/source/sniffer.c</span><br><span>@@ -215,7 +215,7 @@</span><br><span> wt_d = d;</span><br><span> }</span><br><span> wt = wt_wi*960UL*wt_d;</span><br><span style="color: hsl(0, 100%, 40%);">- TRACE_INFO("WT updated to %u\n\r", wt);</span><br><span style="color: hsl(120, 100%, 40%);">+ TRACE_INFO("WT updated to %lu\n\r", wt);</span><br><span> }</span><br><span> </span><br><span> /*! Allocate USB buffer and push + initialize simtrace_msg_hdr</span><br><span>@@ -325,7 +325,7 @@</span><br><span> uint32_t i;</span><br><span> for (i = 0; i < nb_flags; i++) {</span><br><span> if (flags & flag_meanings[i].value) {</span><br><span style="color: hsl(0, 100%, 40%);">- printf(flag_meanings[i].str);</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("%s", flag_meanings[i].str);</span><br><span> flags &= ~flag_meanings[i].value;</span><br><span> if (flags) {</span><br><span> printf(", ");</span><br><span>diff --git a/firmware/libcommon/source/stdio.c b/firmware/libcommon/source/stdio.c</span><br><span>index 06ad611..04d73f0 100644</span><br><span>--- a/firmware/libcommon/source/stdio.c</span><br><span>+++ b/firmware/libcommon/source/stdio.c</span><br><span>@@ -62,8 +62,10 @@</span><br><span> //------------------------------------------------------------------------------</span><br><span> //</span><br><span> FILE* const stdin = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">-FILE* const stdout = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">-FILE* const stderr = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+/* If we use NULL here, we get compiler warnings of calling stdio functions with</span><br><span style="color: hsl(120, 100%, 40%);">+ * NULL values. Our fputs() implementation ignores the value of those pointers anyway */</span><br><span style="color: hsl(120, 100%, 40%);">+FILE* const stdout = (FILE *) 0x1;</span><br><span style="color: hsl(120, 100%, 40%);">+FILE* const stderr = (FILE *) 0x2;</span><br><span> </span><br><span> </span><br><span> //------------------------------------------------------------------------------</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/10608">change 10608</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/10608"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: simtrace2 </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I5ec466361bcc526fac1f4897673264ee5af3458b </div>
<div style="display:none"> Gerrit-Change-Number: 10608 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>