| Message ID | 1453751758-22867-2-git-send-email-mauricio.vasquezbernal@studenti.polito.it |
|---|---|
| State | Superseded |
| Headers | show |
Apologies for the delay in the review. I think this and the previous patch could benefit from using str_to_uint(). Would you mind making that change and resubmitting? I'll be happy to apply it then. Thanks, Daniele On 25/01/2016 11:55, "dev on behalf of Mauricio Vasquez B" <dev-bounces@openvswitch.org on behalf of mauricio.vasquezbernal@studenti.polito.it> wrote: >Current implementation of the ring_client test only supports up to the >dpdkr255 port, this patch extends it to support the full range of possible >dpdkr ports. > >Signed-off-by: Mauricio Vasquez B ><mauricio.vasquezbernal@studenti.polito.it> >--- >v2: >- replace strtol by strtoul >Thanks to Aaron Conole! > tests/dpdk/ring_client.c | 44 >+++++++++++++++++++++++++++----------------- > 1 file changed, 27 insertions(+), 17 deletions(-) > >diff --git a/tests/dpdk/ring_client.c b/tests/dpdk/ring_client.c >index aeaeaca..d02c60b 100644 >--- a/tests/dpdk/ring_client.c >+++ b/tests/dpdk/ring_client.c >@@ -33,6 +33,10 @@ > */ > > #include <getopt.h> >+#include <stdlib.h> >+#include <ctype.h> >+#include <errno.h> >+#include <limits.h> > > #include <config.h> > #include <rte_config.h> >@@ -56,20 +60,18 @@ > /* Our client id number - tells us which rx queue to read, and tx > * queue to write to. > */ >-static uint8_t client_id = 0; >+static unsigned int client_id; > > /* > * Given the rx queue name template above, get the queue name. > */ > static inline const char * >-get_rx_queue_name(unsigned id) >+get_rx_queue_name(unsigned int id) > { >- /* Buffer for return value. Size calculated by %u being replaced >- * by maximum 3 digits (plus an extra byte for safety). >- */ >- static char buffer[sizeof(MP_CLIENT_RXQ_NAME) + 2]; >+ /* Buffer for return value. */ >+ static char buffer[RTE_RING_NAMESIZE]; > >- snprintf(buffer, sizeof(buffer) - 1, MP_CLIENT_RXQ_NAME, id); >+ snprintf(buffer, sizeof(buffer), MP_CLIENT_RXQ_NAME, id); > return buffer; > } > >@@ -77,14 +79,12 @@ get_rx_queue_name(unsigned id) > * Given the tx queue name template above, get the queue name. > */ > static inline const char * >-get_tx_queue_name(unsigned id) >+get_tx_queue_name(unsigned int id) > { >- /* Buffer for return value. Size calculated by %u being replaced >- * by maximum 3 digits (plus an extra byte for safety). >- */ >- static char buffer[sizeof(MP_CLIENT_TXQ_NAME) + 2]; >+ /* Buffer for return value. */ >+ static char buffer[RTE_RING_NAMESIZE]; > >- snprintf(buffer, sizeof(buffer) - 1, MP_CLIENT_TXQ_NAME, id); >+ snprintf(buffer, sizeof(buffer), MP_CLIENT_TXQ_NAME, id); > return buffer; > } > >@@ -98,7 +98,7 @@ usage(const char *progname) > } > > /* >- * Convert the client id number from a string to an int. >+ * Convert the client id number from a string to an usigned int. > */ > static int > parse_client_num(const char *client) >@@ -106,18 +106,28 @@ parse_client_num(const char *client) > char *end = NULL; > unsigned long temp = 0; > >- if (client == NULL || *client == '\0') { >+ if (client == NULL || !isdigit(client[0])) { > return -1; > } > >+ errno = 0; > temp = strtoul(client, &end, BASE_10); >+ >+ if(errno != 0) { >+ return -1; >+ } >+ > /* If valid string argument is provided, terminating '/0' character > * is stored in 'end'. */ >- if (end == NULL || *end != '\0') { >+ if(end == NULL || *end != '\0' || end == client) { >+ return -1; >+ } >+ >+ if(temp > UINT_MAX) { > return -1; > } > >- client_id = (uint8_t)temp; >+ client_id = (unsigned int)temp; > return 0; > } > >-- >1.9.1 > >_______________________________________________ >dev mailing list >dev@openvswitch.org >http://openvswitch.org/mailman/listinfo/dev
Hi Daniele, I didn't know about that function, thanks!. I already sent the new versions: http://openvswitch.org/pipermail/dev/2016-February/066650.html http://openvswitch.org/pipermail/dev/2016-February/066649.html Thank you very much!. On 23 February 2016 at 19:12, Daniele Di Proietto <diproiettod@vmware.com> wrote: > Apologies for the delay in the review. > > I think this and the previous patch could benefit from using str_to_uint(). > > Would you mind making that change and resubmitting? I'll be happy to apply > it then. > > Thanks, > > Daniele > > On 25/01/2016 11:55, "dev on behalf of Mauricio Vasquez B" > <dev-bounces@openvswitch.org on behalf of > mauricio.vasquezbernal@studenti.polito.it> wrote: > > >Current implementation of the ring_client test only supports up to the > >dpdkr255 port, this patch extends it to support the full range of possible > >dpdkr ports. > > > >Signed-off-by: Mauricio Vasquez B > ><mauricio.vasquezbernal@studenti.polito.it> > >--- > >v2: > >- replace strtol by strtoul > >Thanks to Aaron Conole! > > tests/dpdk/ring_client.c | 44 > >+++++++++++++++++++++++++++----------------- > > 1 file changed, 27 insertions(+), 17 deletions(-) > > > >diff --git a/tests/dpdk/ring_client.c b/tests/dpdk/ring_client.c > >index aeaeaca..d02c60b 100644 > >--- a/tests/dpdk/ring_client.c > >+++ b/tests/dpdk/ring_client.c > >@@ -33,6 +33,10 @@ > > */ > > > > #include <getopt.h> > >+#include <stdlib.h> > >+#include <ctype.h> > >+#include <errno.h> > >+#include <limits.h> > > > > #include <config.h> > > #include <rte_config.h> > >@@ -56,20 +60,18 @@ > > /* Our client id number - tells us which rx queue to read, and tx > > * queue to write to. > > */ > >-static uint8_t client_id = 0; > >+static unsigned int client_id; > > > > /* > > * Given the rx queue name template above, get the queue name. > > */ > > static inline const char * > >-get_rx_queue_name(unsigned id) > >+get_rx_queue_name(unsigned int id) > > { > >- /* Buffer for return value. Size calculated by %u being replaced > >- * by maximum 3 digits (plus an extra byte for safety). > >- */ > >- static char buffer[sizeof(MP_CLIENT_RXQ_NAME) + 2]; > >+ /* Buffer for return value. */ > >+ static char buffer[RTE_RING_NAMESIZE]; > > > >- snprintf(buffer, sizeof(buffer) - 1, MP_CLIENT_RXQ_NAME, id); > >+ snprintf(buffer, sizeof(buffer), MP_CLIENT_RXQ_NAME, id); > > return buffer; > > } > > > >@@ -77,14 +79,12 @@ get_rx_queue_name(unsigned id) > > * Given the tx queue name template above, get the queue name. > > */ > > static inline const char * > >-get_tx_queue_name(unsigned id) > >+get_tx_queue_name(unsigned int id) > > { > >- /* Buffer for return value. Size calculated by %u being replaced > >- * by maximum 3 digits (plus an extra byte for safety). > >- */ > >- static char buffer[sizeof(MP_CLIENT_TXQ_NAME) + 2]; > >+ /* Buffer for return value. */ > >+ static char buffer[RTE_RING_NAMESIZE]; > > > >- snprintf(buffer, sizeof(buffer) - 1, MP_CLIENT_TXQ_NAME, id); > >+ snprintf(buffer, sizeof(buffer), MP_CLIENT_TXQ_NAME, id); > > return buffer; > > } > > > >@@ -98,7 +98,7 @@ usage(const char *progname) > > } > > > > /* > >- * Convert the client id number from a string to an int. > >+ * Convert the client id number from a string to an usigned int. > > */ > > static int > > parse_client_num(const char *client) > >@@ -106,18 +106,28 @@ parse_client_num(const char *client) > > char *end = NULL; > > unsigned long temp = 0; > > > >- if (client == NULL || *client == '\0') { > >+ if (client == NULL || !isdigit(client[0])) { > > return -1; > > } > > > >+ errno = 0; > > temp = strtoul(client, &end, BASE_10); > >+ > >+ if(errno != 0) { > >+ return -1; > >+ } > >+ > > /* If valid string argument is provided, terminating '/0' character > > * is stored in 'end'. */ > >- if (end == NULL || *end != '\0') { > >+ if(end == NULL || *end != '\0' || end == client) { > >+ return -1; > >+ } > >+ > >+ if(temp > UINT_MAX) { > > return -1; > > } > > > >- client_id = (uint8_t)temp; > >+ client_id = (unsigned int)temp; > > return 0; > > } > > > >-- > >1.9.1 > > > >_______________________________________________ > >dev mailing list > >dev@openvswitch.org > >http://openvswitch.org/mailman/listinfo/dev > >
diff --git a/tests/dpdk/ring_client.c b/tests/dpdk/ring_client.c index aeaeaca..d02c60b 100644 --- a/tests/dpdk/ring_client.c +++ b/tests/dpdk/ring_client.c @@ -33,6 +33,10 @@ */ #include <getopt.h> +#include <stdlib.h> +#include <ctype.h> +#include <errno.h> +#include <limits.h> #include <config.h> #include <rte_config.h> @@ -56,20 +60,18 @@ /* Our client id number - tells us which rx queue to read, and tx * queue to write to. */ -static uint8_t client_id = 0; +static unsigned int client_id; /* * Given the rx queue name template above, get the queue name. */ static inline const char * -get_rx_queue_name(unsigned id) +get_rx_queue_name(unsigned int id) { - /* Buffer for return value. Size calculated by %u being replaced - * by maximum 3 digits (plus an extra byte for safety). - */ - static char buffer[sizeof(MP_CLIENT_RXQ_NAME) + 2]; + /* Buffer for return value. */ + static char buffer[RTE_RING_NAMESIZE]; - snprintf(buffer, sizeof(buffer) - 1, MP_CLIENT_RXQ_NAME, id); + snprintf(buffer, sizeof(buffer), MP_CLIENT_RXQ_NAME, id); return buffer; } @@ -77,14 +79,12 @@ get_rx_queue_name(unsigned id) * Given the tx queue name template above, get the queue name. */ static inline const char * -get_tx_queue_name(unsigned id) +get_tx_queue_name(unsigned int id) { - /* Buffer for return value. Size calculated by %u being replaced - * by maximum 3 digits (plus an extra byte for safety). - */ - static char buffer[sizeof(MP_CLIENT_TXQ_NAME) + 2]; + /* Buffer for return value. */ + static char buffer[RTE_RING_NAMESIZE]; - snprintf(buffer, sizeof(buffer) - 1, MP_CLIENT_TXQ_NAME, id); + snprintf(buffer, sizeof(buffer), MP_CLIENT_TXQ_NAME, id); return buffer; } @@ -98,7 +98,7 @@ usage(const char *progname) } /* - * Convert the client id number from a string to an int. + * Convert the client id number from a string to an usigned int. */ static int parse_client_num(const char *client) @@ -106,18 +106,28 @@ parse_client_num(const char *client) char *end = NULL; unsigned long temp = 0; - if (client == NULL || *client == '\0') { + if (client == NULL || !isdigit(client[0])) { return -1; } + errno = 0; temp = strtoul(client, &end, BASE_10); + + if(errno != 0) { + return -1; + } + /* If valid string argument is provided, terminating '/0' character * is stored in 'end'. */ - if (end == NULL || *end != '\0') { + if(end == NULL || *end != '\0' || end == client) { + return -1; + } + + if(temp > UINT_MAX) { return -1; } - client_id = (uint8_t)temp; + client_id = (unsigned int)temp; return 0; }
Current implementation of the ring_client test only supports up to the dpdkr255 port, this patch extends it to support the full range of possible dpdkr ports. Signed-off-by: Mauricio Vasquez B <mauricio.vasquezbernal@studenti.polito.it> --- v2: - replace strtol by strtoul Thanks to Aaron Conole! tests/dpdk/ring_client.c | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-)