From c394109964a18d05d46de46728f617465f3493c4 Mon Sep 17 00:00:00 2001 From: Harald Welte Date: Sun, 26 Aug 2018 09:53:13 +0200 Subject: 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 --- firmware/Makefile | 3 +-- firmware/apps/cardem/main.c | 4 ++-- firmware/apps/trace/main.c | 4 ++-- firmware/libboard/common/source/boardver_adc.c | 2 +- firmware/libboard/qmod/source/board_qmod.c | 4 ++-- firmware/libcommon/source/card_emu.c | 2 +- firmware/libcommon/source/host_communication.c | 2 +- firmware/libcommon/source/iso7816_4.c | 4 ++-- firmware/libcommon/source/mode_cardemu.c | 8 ++++---- firmware/libcommon/source/pseudo_talloc.c | 2 +- firmware/libcommon/source/simtrace_iso7816.c | 2 +- firmware/libcommon/source/sniffer.c | 4 ++-- firmware/libcommon/source/stdio.c | 6 ++++-- 13 files changed, 24 insertions(+), 23 deletions(-) diff --git a/firmware/Makefile b/firmware/Makefile index c9039a3..c0f53f3 100644 --- a/firmware/Makefile +++ b/firmware/Makefile @@ -141,13 +141,12 @@ INCLUDES += -Ilibosmocore/include 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 @@ extern int main(void) "=============================================================================\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 @@ extern int main(void) 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 @@ extern int main(void) "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 @@ int get_board_version_adc(void) /* 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 @@ static void board_exec_dbg_cmd_st12only(int ch) 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 @@ enum tpdu_state { #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 @@ int usb_refill_from_host(uint8_t ep) 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 @@ uint32_t ISO7816_SendChar( uint8_t CharToSend, Usart_info *usart ) 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 @@ static const Pin pin_usim2_vcc = PIN_USIM2_VCC; #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 @@ static void wait_tx_idle(Usart *usart) /* 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 card_emu_uart_tx(uint8_t uart_chan, uint8_t byte) 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 @@ static void usart_irq_rx(uint8_t inst_num) 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 @@ int _talloc_free(void *ptr, const char *location) 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 @@ void ISR_PhoneRST(const Pin * pPin) { 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 @@ static void update_wt(uint8_t wi, uint8_t d) 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 @@ static void print_flags(const struct value_string* flag_meanings, uint32_t nb_fl 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; //------------------------------------------------------------------------------ -- cgit v1.2.3