Mauricio Vasquez B <mauricio.vasquezber...@studenti.polito.it> writes:
> Current implementation of dpdk_dev_parse_name does not perform a robust
> error handling, port names as "dpdkr" or "dpdkr1x" are considered valid.
Mauricio, thanks for the patch!
> Signed-off-by: Mauricio Vasquez B <mauricio.vasquezber...@studenti.polito.it>
> ---
> lib/netdev-dpdk.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index de7e488..ac81f2f 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -19,6 +19,7 @@
> #include <string.h>
> #include <signal.h>
> #include <stdlib.h>
> +#include <limits.h>
> #include <pthread.h>
> #include <config.h>
> #include <errno.h>
> @@ -187,7 +188,7 @@ struct dpdk_ring {
> /* For the client rings */
> struct rte_ring *cring_tx;
> struct rte_ring *cring_rx;
> - int user_port_id; /* User given port no, parsed from port name */
> + unsigned int user_port_id; /* User given port no, parsed from port name
> */
> int eth_port_id; /* ethernet device port id */
> struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
> };
> @@ -641,13 +642,30 @@ dpdk_dev_parse_name(const char dev_name[], const char
> prefix[],
> unsigned int *port_no)
> {
> const char *cport;
> + unsigned long port;
port here is unsigned, but you use it with strtol (I know the original
code did this as well - just curious). Also, why introduce it at all? Is
it just to avoid assigning port_no on error? If so, I suggest using long
type and check it's value for less-than 0 as well (so that dpdkr-123
would also be an error).
> + char *endptr;
>
> if (strncmp(dev_name, prefix, strlen(prefix))) {
> return ENODEV;
> }
>
> + errno = 0;
> cport = dev_name + strlen(prefix);
> - *port_no = strtol(cport, NULL, 0); /* string must be null terminated */
> + port = strtol(cport, &endptr, 10);
Would there be any value in a non-decimal input on the port number ever?
Just curious, since I also don't see one.
> + if(errno != 0) {
> + return errno;
> + }
> +
> + if(endptr == NULL || *endptr != '\0' || endptr == cport) {
> + return ENODEV;
> + }
> +
> + if(port > UINT_MAX) {
> + return ENODEV;
> + }
> +
> + *port_no = port;
> return 0;
> }
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
