From cc03f84636c03d6a06d47dd3ca84fe2ace04fea8 Mon Sep 17 00:00:00 2001 From: Philippe Teuwen Date: Sun, 10 Mar 2013 00:20:52 +0100 Subject: [PATCH] New connstring_decode() fix cppcheck warning "Non reentrant function 'strtok' called" --- libnfc/drivers/acr122_pcsc.c | 54 +++++++------------------- libnfc/drivers/acr122_usb.c | 46 +--------------------- libnfc/drivers/acr122s.c | 70 +++++++++------------------------ libnfc/drivers/arygon.c | 73 ++++++++++------------------------- libnfc/drivers/pn532_uart.c | 75 ++++++++++-------------------------- libnfc/drivers/pn53x_usb.c | 46 +--------------------- libnfc/nfc-internal.c | 64 ++++++++++++++++++++++++++++++ libnfc/nfc-internal.h | 2 + 8 files changed, 140 insertions(+), 290 deletions(-) diff --git a/libnfc/drivers/acr122_pcsc.c b/libnfc/drivers/acr122_pcsc.c index 4341aad..a60825c 100644 --- a/libnfc/drivers/acr122_pcsc.c +++ b/libnfc/drivers/acr122_pcsc.c @@ -191,49 +191,14 @@ acr122_pcsc_scan(const nfc_context *context, nfc_connstring connstrings[], const } struct acr122_pcsc_descriptor { - char pcsc_device_name[512]; + char *pcsc_device_name; }; -static int -acr122_pcsc_connstring_decode(const nfc_connstring connstring, struct acr122_pcsc_descriptor *desc) -{ - char *cs = malloc(strlen(connstring) + 1); - if (!cs) { - perror("malloc"); - return -1; - } - strcpy(cs, connstring); - const char *driver_name = strtok(cs, ":"); - if (!driver_name) { - // Parse error - free(cs); - return -1; - } - - if (0 != strcmp(driver_name, ACR122_PCSC_DRIVER_NAME)) { - // Driver name does not match. - free(cs); - return 0; - } - - const char *device_name = strtok(NULL, ":"); - if (!device_name) { - // Only driver name was specified (or parsing error) - free(cs); - return 1; - } - strncpy(desc->pcsc_device_name, device_name, sizeof(desc->pcsc_device_name) - 1); - desc->pcsc_device_name[sizeof(desc->pcsc_device_name) - 1] = '\0'; - - free(cs); - return 2; -} - static nfc_device * acr122_pcsc_open(const nfc_context *context, const nfc_connstring connstring) { struct acr122_pcsc_descriptor ndd; - int connstring_decode_level = acr122_pcsc_connstring_decode(connstring, &ndd); + int connstring_decode_level = connstring_decode(connstring, ACR122_PCSC_DRIVER_NAME, "pcsc", &ndd.pcsc_device_name, NULL); if (connstring_decode_level < 1) { return NULL; @@ -245,7 +210,7 @@ acr122_pcsc_open(const nfc_context *context, const nfc_connstring connstring) size_t szDeviceFound = acr122_pcsc_scan(context, &fullconnstring, 1); if (szDeviceFound < 1) return NULL; - connstring_decode_level = acr122_pcsc_connstring_decode(fullconnstring, &ndd); + connstring_decode_level = connstring_decode(fullconnstring, ACR122_PCSC_DRIVER_NAME, "pcsc", &ndd.pcsc_device_name, NULL); if (connstring_decode_level < 2) { return NULL; } @@ -255,23 +220,29 @@ acr122_pcsc_open(const nfc_context *context, const nfc_connstring connstring) if (strlen(ndd.pcsc_device_name) < 5) { // We can assume it's a reader ID as pcsc_name always ends with "NN NN" // Device was not specified, only ID, retrieve it size_t index; - if (sscanf(ndd.pcsc_device_name, "%4" SCNuPTR, &index) != 1) + if (sscanf(ndd.pcsc_device_name, "%4" SCNuPTR, &index) != 1) { + free(ndd.pcsc_device_name); return NULL; + } nfc_connstring *ncs = malloc(sizeof(nfc_connstring) * (index + 1)); if (!ncs) { perror("malloc"); + free(ndd.pcsc_device_name); return NULL; } size_t szDeviceFound = acr122_pcsc_scan(context, ncs, index + 1); if (szDeviceFound < index + 1) { free(ncs); + free(ndd.pcsc_device_name); return NULL; } strncpy(fullconnstring, ncs[index], sizeof(nfc_connstring)); fullconnstring[sizeof(nfc_connstring) - 1] = '\0'; free(ncs); - connstring_decode_level = acr122_pcsc_connstring_decode(fullconnstring, &ndd); + connstring_decode_level = connstring_decode(fullconnstring, ACR122_PCSC_DRIVER_NAME, "pcsc", &ndd.pcsc_device_name, NULL); + if (connstring_decode_level < 2) { + free(ndd.pcsc_device_name); return NULL; } } @@ -324,12 +295,13 @@ acr122_pcsc_open(const nfc_context *context, const nfc_connstring connstring) pn53x_init(pnd); + free(ndd.pcsc_device_name); return pnd; } error: + free(ndd.pcsc_device_name); nfc_device_free(pnd); - return NULL; } diff --git a/libnfc/drivers/acr122_usb.c b/libnfc/drivers/acr122_usb.c index 93e65f3..e4ccae2 100644 --- a/libnfc/drivers/acr122_usb.c +++ b/libnfc/drivers/acr122_usb.c @@ -347,50 +347,6 @@ struct acr122_usb_descriptor { char *filename; }; -static int -acr122_usb_connstring_decode(const nfc_connstring connstring, struct acr122_usb_descriptor *desc) -{ - int n = strlen(connstring) + 1; - char *driver_name = malloc(n); - if (!driver_name) { - perror("malloc"); - return 0; - } - char *dirname = malloc(n); - if (!dirname) { - perror("malloc"); - free(driver_name); - return 0; - } - char *filename = malloc(n); - if (!filename) { - perror("malloc"); - free(driver_name); - free(dirname); - return 0; - } - - driver_name[0] = '\0'; - - char format[32]; - snprintf(format, sizeof(format), "%%%i[^:]:%%%i[^:]:%%%i[^:]", n - 1, n - 1, n - 1); - int res = sscanf(connstring, format, driver_name, dirname, filename); - - if (!res || ((0 != strcmp(driver_name, ACR122_USB_DRIVER_NAME)) && (0 != strcmp(driver_name, "usb")))) { - // Driver name does not match. - res = 0; - } else { - desc->dirname = strdup(dirname); - desc->filename = strdup(filename); - } - - free(driver_name); - free(dirname); - free(filename); - - return res; -} - static bool acr122_usb_get_usb_device_name(struct usb_device *dev, usb_dev_handle *udev, char *buffer, size_t len) { @@ -424,7 +380,7 @@ acr122_usb_open(const nfc_context *context, const nfc_connstring connstring) { nfc_device *pnd = NULL; struct acr122_usb_descriptor desc = { NULL, NULL }; - int connstring_decode_level = acr122_usb_connstring_decode(connstring, &desc); + int connstring_decode_level = connstring_decode(connstring, ACR122_USB_DRIVER_NAME, "usb", &desc.dirname, &desc.filename); log_put(LOG_GROUP, LOG_CATEGORY, NFC_LOG_PRIORITY_DEBUG, "%d element(s) have been decoded from \"%s\"", connstring_decode_level, connstring); if (connstring_decode_level < 1) { goto free_mem; diff --git a/libnfc/drivers/acr122s.c b/libnfc/drivers/acr122s.c index 3daf30b..f6e69e1 100644 --- a/libnfc/drivers/acr122s.c +++ b/libnfc/drivers/acr122s.c @@ -396,59 +396,10 @@ acr122s_get_firmware_version(nfc_device *pnd, char *version, size_t length) } struct acr122s_descriptor { - char port[128]; + char *port; uint32_t speed; }; -static int -acr122s_connstring_decode(const nfc_connstring connstring, struct acr122s_descriptor *desc) -{ - char *cs = malloc(strlen(connstring) + 1); - if (!cs) { - perror("malloc"); - return -1; - } - strcpy(cs, connstring); - const char *driver_name = strtok(cs, ":"); - if (!driver_name) { - // Parse error - free(cs); - return -1; - } - - if (0 != strcmp(driver_name, ACR122S_DRIVER_NAME)) { - // Driver name does not match. - free(cs); - return 0; - } - - const char *port = strtok(NULL, ":"); - if (!port) { - // Only driver name was specified (or parsing error) - free(cs); - return 1; - } - strncpy(desc->port, port, sizeof(desc->port) - 1); - desc->port[sizeof(desc->port) - 1] = '\0'; - - const char *speed_s = strtok(NULL, ":"); - if (!speed_s) { - // speed not specified (or parsing error) - free(cs); - return 2; - } - unsigned long speed; - if (sscanf(speed_s, "%10lu", &speed) != 1) { - // speed_s is not a number - free(cs); - return 2; - } - desc->speed = speed; - - free(cs); - return 3; -} - static size_t acr122s_scan(const nfc_context *context, nfc_connstring connstrings[], const size_t connstrings_len) { @@ -540,6 +491,7 @@ acr122s_close(nfc_device *pnd) close(DRIVER_DATA(pnd)->abort_fds[1]); #endif + free(DRIVER_DATA(pnd)->port); pn53x_data_free(pnd); nfc_device_free(pnd); } @@ -550,8 +502,18 @@ acr122s_open(const nfc_context *context, const nfc_connstring connstring) serial_port sp; nfc_device *pnd; struct acr122s_descriptor ndd; - int connstring_decode_level = acr122s_connstring_decode(connstring, &ndd); - + char *speed_s; + int connstring_decode_level = connstring_decode(connstring, ACR122S_DRIVER_NAME, NULL, &ndd.port, &speed_s); + if (connstring_decode_level == 3) { + ndd.speed = 0; + if (sscanf(speed_s, "%10"PRIu32, &ndd.speed) != 1) { + // speed_s is not a number + free(ndd.port); + free(speed_s); + return NULL; + } + free(speed_s); + } if (connstring_decode_level < 2) { return NULL; } @@ -566,11 +528,13 @@ acr122s_open(const nfc_context *context, const nfc_connstring connstring) if (sp == INVALID_SERIAL_PORT) { log_put(LOG_GROUP, LOG_CATEGORY, NFC_LOG_PRIORITY_ERROR, "Invalid serial port: %s", ndd.port); + free(ndd.port); return NULL; } if (sp == CLAIMED_SERIAL_PORT) { log_put(LOG_GROUP, LOG_CATEGORY, NFC_LOG_PRIORITY_ERROR, "Serial port already claimed: %s", ndd.port); + free(ndd.port); return NULL; } @@ -580,6 +544,7 @@ acr122s_open(const nfc_context *context, const nfc_connstring connstring) pnd = nfc_device_new(context, connstring); if (!pnd) { perror("malloc"); + acr122s_close(pnd); return NULL; } pnd->driver = &acr122s_driver; @@ -597,6 +562,7 @@ acr122s_open(const nfc_context *context, const nfc_connstring connstring) #ifndef WIN32 if (pipe(DRIVER_DATA(pnd)->abort_fds) < 0) { + acr122s_close(pnd); return NULL; } #else diff --git a/libnfc/drivers/arygon.c b/libnfc/drivers/arygon.c index 729b0b8..d7f78d7 100644 --- a/libnfc/drivers/arygon.c +++ b/libnfc/drivers/arygon.c @@ -165,59 +165,10 @@ arygon_scan(const nfc_context *context, nfc_connstring connstrings[], const size } struct arygon_descriptor { - char port[128]; + char *port; uint32_t speed; }; -static int -arygon_connstring_decode(const nfc_connstring connstring, struct arygon_descriptor *desc) -{ - char *cs = malloc(strlen(connstring) + 1); - if (!cs) { - perror("malloc"); - return -1; - } - strcpy(cs, connstring); - const char *driver_name = strtok(cs, ":"); - if (!driver_name) { - // Parse error - free(cs); - return -1; - } - - if (0 != strcmp(driver_name, ARYGON_DRIVER_NAME)) { - // Driver name does not match. - free(cs); - return 0; - } - - const char *port = strtok(NULL, ":"); - if (!port) { - // Only driver name was specified (or parsing error) - free(cs); - return 1; - } - strncpy(desc->port, port, sizeof(desc->port) - 1); - desc->port[sizeof(desc->port) - 1] = '\0'; - - const char *speed_s = strtok(NULL, ":"); - if (!speed_s) { - // speed not specified (or parsing error) - free(cs); - return 2; - } - unsigned long speed; - if (sscanf(speed_s, "%10lu", &speed) != 1) { - // speed_s is not a number - free(cs); - return 2; - } - desc->speed = speed; - - free(cs); - return 3; -} - static void arygon_close(nfc_device *pnd) { @@ -232,6 +183,7 @@ arygon_close(nfc_device *pnd) close(DRIVER_DATA(pnd)->iAbortFds[1]); #endif + free(DRIVER_DATA(pnd)->port); pn53x_data_free(pnd); nfc_device_free(pnd); } @@ -240,8 +192,18 @@ static nfc_device * arygon_open(const nfc_context *context, const nfc_connstring connstring) { struct arygon_descriptor ndd; - int connstring_decode_level = arygon_connstring_decode(connstring, &ndd); - + char *speed_s; + int connstring_decode_level = connstring_decode(connstring, ARYGON_DRIVER_NAME, NULL, &ndd.port, &speed_s); + if (connstring_decode_level == 3) { + ndd.speed = 0; + if (sscanf(speed_s, "%10"PRIu32, &ndd.speed) != 1) { + // speed_s is not a number + free(ndd.port); + free(speed_s); + return NULL; + } + free(speed_s); + } if (connstring_decode_level < 2) { return NULL; } @@ -258,8 +220,10 @@ arygon_open(const nfc_context *context, const nfc_connstring connstring) log_put(LOG_GROUP, LOG_CATEGORY, NFC_LOG_PRIORITY_ERROR, "Invalid serial port: %s", ndd.port); if (sp == CLAIMED_SERIAL_PORT) log_put(LOG_GROUP, LOG_CATEGORY, NFC_LOG_PRIORITY_ERROR, "Serial port already claimed: %s", ndd.port); - if ((sp == CLAIMED_SERIAL_PORT) || (sp == INVALID_SERIAL_PORT)) + if ((sp == CLAIMED_SERIAL_PORT) || (sp == INVALID_SERIAL_PORT)) { + free(ndd.port); return NULL; + } // We need to flush input to be sure first reply does not comes from older byte transceive uart_flush_input(sp); @@ -269,6 +233,7 @@ arygon_open(const nfc_context *context, const nfc_connstring connstring) pnd = nfc_device_new(context, connstring); if (!pnd) { perror("malloc"); + free(ndd.port); return NULL; } snprintf(pnd->name, sizeof(pnd->name), "%s:%s", ARYGON_DRIVER_NAME, ndd.port); @@ -276,6 +241,7 @@ arygon_open(const nfc_context *context, const nfc_connstring connstring) pnd->driver_data = malloc(sizeof(struct arygon_data)); if (!pnd->driver_data) { perror("malloc"); + free(ndd.port); return NULL; } DRIVER_DATA(pnd)->port = sp; @@ -293,6 +259,7 @@ arygon_open(const nfc_context *context, const nfc_connstring connstring) #ifndef WIN32 // pipe-based abort mecanism if (pipe(DRIVER_DATA(pnd)->iAbortFds) < 0) { + free(ndd.port); return NULL; } #else diff --git a/libnfc/drivers/pn532_uart.c b/libnfc/drivers/pn532_uart.c index 6816558..409b2e7 100644 --- a/libnfc/drivers/pn532_uart.c +++ b/libnfc/drivers/pn532_uart.c @@ -142,59 +142,10 @@ pn532_uart_scan(const nfc_context *context, nfc_connstring connstrings[], const } struct pn532_uart_descriptor { - char port[128]; + char *port; uint32_t speed; }; -static int -pn532_connstring_decode(const nfc_connstring connstring, struct pn532_uart_descriptor *desc) -{ - char *cs = malloc(strlen(connstring) + 1); - if (!cs) { - perror("malloc"); - return -1; - } - strcpy(cs, connstring); - const char *driver_name = strtok(cs, ":"); - if (!driver_name) { - // Parse error - free(cs); - return -1; - } - - if (0 != strcmp(driver_name, PN532_UART_DRIVER_NAME)) { - // Driver name does not match. - free(cs); - return 0; - } - - const char *port = strtok(NULL, ":"); - if (!port) { - // Only driver name was specified (or parsing error) - free(cs); - return 1; - } - strncpy(desc->port, port, sizeof(desc->port) - 1); - desc->port[sizeof(desc->port) - 1] = '\0'; - - const char *speed_s = strtok(NULL, ":"); - if (!speed_s) { - // speed not specified (or parsing error) - free(cs); - return 2; - } - unsigned long speed; - if (sscanf(speed_s, "%10lu", &speed) != 1) { - // speed_s is not a number - free(cs); - return 2; - } - desc->speed = speed; - - free(cs); - return 3; -} - static void pn532_uart_close(nfc_device *pnd) { @@ -209,6 +160,7 @@ pn532_uart_close(nfc_device *pnd) close(DRIVER_DATA(pnd)->iAbortFds[1]); #endif + free(DRIVER_DATA(pnd)->port); pn53x_data_free(pnd); nfc_device_free(pnd); } @@ -217,8 +169,18 @@ static nfc_device * pn532_uart_open(const nfc_context *context, const nfc_connstring connstring) { struct pn532_uart_descriptor ndd; - int connstring_decode_level = pn532_connstring_decode(connstring, &ndd); - + char *speed_s; + int connstring_decode_level = connstring_decode(connstring, PN532_UART_DRIVER_NAME, NULL, &ndd.port, &speed_s); + if (connstring_decode_level == 3) { + ndd.speed = 0; + if (sscanf(speed_s, "%10"PRIu32, &ndd.speed) != 1) { + // speed_s is not a number + free(ndd.port); + free(speed_s); + return NULL; + } + free(speed_s); + } if (connstring_decode_level < 2) { return NULL; } @@ -235,9 +197,10 @@ pn532_uart_open(const nfc_context *context, const nfc_connstring connstring) log_put(LOG_GROUP, LOG_CATEGORY, NFC_LOG_PRIORITY_ERROR, "Invalid serial port: %s", ndd.port); if (sp == CLAIMED_SERIAL_PORT) log_put(LOG_GROUP, LOG_CATEGORY, NFC_LOG_PRIORITY_ERROR, "Serial port already claimed: %s", ndd.port); - if ((sp == CLAIMED_SERIAL_PORT) || (sp == INVALID_SERIAL_PORT)) + if ((sp == CLAIMED_SERIAL_PORT) || (sp == INVALID_SERIAL_PORT)) { + free(ndd.port); return NULL; - + } // We need to flush input to be sure first reply does not comes from older byte transceive uart_flush_input(sp); uart_set_speed(sp, ndd.speed); @@ -246,6 +209,7 @@ pn532_uart_open(const nfc_context *context, const nfc_connstring connstring) pnd = nfc_device_new(context, connstring); if (!pnd) { perror("malloc"); + free(ndd.port); return NULL; } snprintf(pnd->name, sizeof(pnd->name), "%s:%s", PN532_UART_DRIVER_NAME, ndd.port); @@ -253,6 +217,7 @@ pn532_uart_open(const nfc_context *context, const nfc_connstring connstring) pnd->driver_data = malloc(sizeof(struct pn532_uart_data)); if (!pnd->driver_data) { perror("malloc"); + free(ndd.port); return NULL; } DRIVER_DATA(pnd)->port = sp; @@ -271,6 +236,7 @@ pn532_uart_open(const nfc_context *context, const nfc_connstring connstring) #ifndef WIN32 // pipe-based abort mecanism if (pipe(DRIVER_DATA(pnd)->iAbortFds) < 0) { + free(ndd.port); return NULL; } #else @@ -281,6 +247,7 @@ pn532_uart_open(const nfc_context *context, const nfc_connstring connstring) if (pn53x_check_communication(pnd) < 0) { nfc_perror(pnd, "pn53x_check_communication"); pn532_uart_close(pnd); + free(ndd.port); return NULL; } diff --git a/libnfc/drivers/pn53x_usb.c b/libnfc/drivers/pn53x_usb.c index edc9eee..6ef3f16 100644 --- a/libnfc/drivers/pn53x_usb.c +++ b/libnfc/drivers/pn53x_usb.c @@ -232,50 +232,6 @@ struct pn53x_usb_descriptor { char *filename; }; -static int -pn53x_usb_connstring_decode(const nfc_connstring connstring, struct pn53x_usb_descriptor *desc) -{ - int n = strlen(connstring) + 1; - char *driver_name = malloc(n); - if (!driver_name) { - perror("malloc"); - return 0; - } - char *dirname = malloc(n); - if (!dirname) { - perror("malloc"); - free(driver_name); - return 0; - } - char *filename = malloc(n); - if (!filename) { - perror("malloc"); - free(driver_name); - free(dirname); - return 0; - } - - driver_name[0] = '\0'; - - char format[32]; - snprintf(format, sizeof(format), "%%%i[^:]:%%%i[^:]:%%%i[^:]", n - 1, n - 1, n - 1); - int res = sscanf(connstring, format, driver_name, dirname, filename); - - if (!res || ((0 != strcmp(driver_name, PN53X_USB_DRIVER_NAME)) && (0 != strcmp(driver_name, "usb")))) { - // Driver name does not match. - res = 0; - } else { - desc->dirname = strdup(dirname); - desc->filename = strdup(filename); - } - - free(driver_name); - free(dirname); - free(filename); - - return res; -} - bool pn53x_usb_get_usb_device_name(struct usb_device *dev, usb_dev_handle *udev, char *buffer, size_t len) { @@ -309,7 +265,7 @@ pn53x_usb_open(const nfc_context *context, const nfc_connstring connstring) { nfc_device *pnd = NULL; struct pn53x_usb_descriptor desc = { NULL, NULL }; - int connstring_decode_level = pn53x_usb_connstring_decode(connstring, &desc); + int connstring_decode_level = connstring_decode(connstring, PN53X_USB_DRIVER_NAME, "usb", &desc.dirname, &desc.filename); log_put(LOG_GROUP, LOG_CATEGORY, NFC_LOG_PRIORITY_DEBUG, "%d element(s) have been decoded from \"%s\"", connstring_decode_level, connstring); if (connstring_decode_level < 1) { goto free_mem; diff --git a/libnfc/nfc-internal.c b/libnfc/nfc-internal.c index 715c7ff..39d7f2e 100644 --- a/libnfc/nfc-internal.c +++ b/libnfc/nfc-internal.c @@ -185,3 +185,67 @@ prepare_initiator_data(const nfc_modulation nm, uint8_t **ppbtInitiatorData, siz break; } } + +int +connstring_decode(const nfc_connstring connstring, const char *driver_name, const char *bus_name, char **pparam1, char **pparam2) +{ + if (driver_name == NULL) { + driver_name = ""; + } + if (bus_name == NULL) { + bus_name = ""; + } + int n = strlen(connstring) + 1; + char *param0 = malloc(n); + if (param0 == NULL) { + perror("malloc"); + return 0; + } + char *param1 = malloc(n); + if (param1 == NULL) { + perror("malloc"); + free(param0); + return 0; + } + char *param2 = malloc(n); + if (param2 == NULL) { + perror("malloc"); + free(param0); + free(param1); + return 0; + } + + char format[32]; + snprintf(format, sizeof(format), "%%%i[^:]:%%%i[^:]:%%%i[^:]", n - 1, n - 1, n - 1); + int res = sscanf(connstring, format, param0, param1, param2); + + if (res < 1 || ((0 != strcmp(param0, driver_name)) && + (bus_name != NULL) && + (0 != strcmp(param0, bus_name)))) { + // Driver name does not match. + res = 0; + } + if (pparam1 != NULL) { + if (res < 2) { + free(param1); + *pparam1 = NULL; + } else { + *pparam1 = param1; + } + } else { + free(param1); + } + if (pparam2 != NULL) { + if (res < 3) { + free(param2); + *pparam2 = NULL; + } else { + *pparam2 = param2; + } + } else { + free(param2); + } + free(param0); + return res; +} + diff --git a/libnfc/nfc-internal.h b/libnfc/nfc-internal.h index 1a5eec4..f0f396d 100644 --- a/libnfc/nfc-internal.h +++ b/libnfc/nfc-internal.h @@ -215,4 +215,6 @@ void iso14443_cascade_uid(const uint8_t abtUID[], const size_t szUID, uint8_t *p void prepare_initiator_data(const nfc_modulation nm, uint8_t **ppbtInitiatorData, size_t *pszInitiatorData); +int connstring_decode(const nfc_connstring connstring, const char *driver_name, const char *bus_name, char **pparam1, char **pparam2); + #endif // __NFC_INTERNAL_H__