[dpdk-dev] devargs: restore empty devargs as ""

Message ID 1424770891-19243-1-git-send-email-david.marchand@6wind.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

David Marchand Feb. 24, 2015, 9:41 a.m. UTC
  Following commit c07691ae1089, an implicit change has been done in the devargs
api.
This triggers problem in virtual pmds that did not check for parameters validity
as it was implicitely valid.

Fix this by restoring the empty argument as "" and add a note in the api.
Restore associated tests.

Fixes: c07691ae1089 ("devargs: remove limit on parameters length")
Reported-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 app/test/test_devargs.c                     |    2 +-
 lib/librte_eal/common/eal_common_devargs.c  |   11 +++++++----
 lib/librte_eal/common/include/rte_devargs.h |    2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)
  

Comments

Tetsuya Mukawa Feb. 24, 2015, 10:16 a.m. UTC | #1
On 2015/02/24 18:41, David Marchand wrote:
> Following commit c07691ae1089, an implicit change has been done in the devargs
> api.
> This triggers problem in virtual pmds that did not check for parameters validity
> as it was implicitely valid.
>
> Fix this by restoring the empty argument as "" and add a note in the api.
> Restore associated tests.
>
> Fixes: c07691ae1089 ("devargs: remove limit on parameters length")
> Reported-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> Signed-off-by: David Marchand <david.marchand@6wind.com>
> ---
>  app/test/test_devargs.c                     |    2 +-
>  lib/librte_eal/common/eal_common_devargs.c  |   11 +++++++----
>  lib/librte_eal/common/include/rte_devargs.h |    2 +-
>  3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/app/test/test_devargs.c b/app/test/test_devargs.c
> index 08fb781..f7fc59c 100644
> --- a/app/test/test_devargs.c
> +++ b/app/test/test_devargs.c
> @@ -107,7 +107,7 @@ test_devargs(void)
>  		devargs->pci.addr.devid != 0 ||
>  		devargs->pci.addr.function != 1)
>  		goto fail;
> -	if (devargs->args)
> +	if (!devargs->args || strcmp(devargs->args, "") != 0)
>  		goto fail;
>  	free_devargs_list();
>  
> diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> index 3aace08..eadd719 100644
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> @@ -73,10 +73,13 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
>  	if (sep != NULL) {
>  		sep[0] = '\0';
>  		devargs->args = strdup(sep + 1);
> -		if (devargs->args == NULL) {
> -			RTE_LOG(ERR, EAL, "cannot allocate for devargs args\n");
> -			goto fail;
> -		}
> +	} else {
> +		devargs->args = strdup("");
> +	}
> +
> +	if (devargs->args == NULL) {
> +		RTE_LOG(ERR, EAL, "cannot allocate for devargs args\n");
> +		goto fail;
>  	}
>  
>  	switch (devargs->type) {
> diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
> index 996e180..6834333 100644
> --- a/lib/librte_eal/common/include/rte_devargs.h
> +++ b/lib/librte_eal/common/include/rte_devargs.h
> @@ -88,7 +88,7 @@ struct rte_devargs {
>  			char drv_name[32];
>  		} virtual;
>  	};
> -	/** Arguments string as given by user. */
> +	/** Arguments string as given by user or "" for no argument. */
>  	char *args;
>  };
>  

Hi David,

I've confirmed this patch fixes the issue descried in above comment.

Thanks,
Tetsuya
  
Thomas Monjalon Feb. 24, 2015, 7:24 p.m. UTC | #2
2015-02-24 19:16, Tetsuya Mukawa:
> On 2015/02/24 18:41, David Marchand wrote:
> > Following commit c07691ae1089, an implicit change has been done in the devargs
> > api.
> > This triggers problem in virtual pmds that did not check for parameters validity
> > as it was implicitely valid.
> >
> > Fix this by restoring the empty argument as "" and add a note in the api.
> > Restore associated tests.
> >
> > Fixes: c07691ae1089 ("devargs: remove limit on parameters length")
> > Reported-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> > Signed-off-by: David Marchand <david.marchand@6wind.com>
> 
> Hi David,
> 
> I've confirmed this patch fixes the issue descried in above comment.

Applied, thanks
  

Patch

diff --git a/app/test/test_devargs.c b/app/test/test_devargs.c
index 08fb781..f7fc59c 100644
--- a/app/test/test_devargs.c
+++ b/app/test/test_devargs.c
@@ -107,7 +107,7 @@  test_devargs(void)
 		devargs->pci.addr.devid != 0 ||
 		devargs->pci.addr.function != 1)
 		goto fail;
-	if (devargs->args)
+	if (!devargs->args || strcmp(devargs->args, "") != 0)
 		goto fail;
 	free_devargs_list();
 
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 3aace08..eadd719 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -73,10 +73,13 @@  rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 	if (sep != NULL) {
 		sep[0] = '\0';
 		devargs->args = strdup(sep + 1);
-		if (devargs->args == NULL) {
-			RTE_LOG(ERR, EAL, "cannot allocate for devargs args\n");
-			goto fail;
-		}
+	} else {
+		devargs->args = strdup("");
+	}
+
+	if (devargs->args == NULL) {
+		RTE_LOG(ERR, EAL, "cannot allocate for devargs args\n");
+		goto fail;
 	}
 
 	switch (devargs->type) {
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 996e180..6834333 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -88,7 +88,7 @@  struct rte_devargs {
 			char drv_name[32];
 		} virtual;
 	};
-	/** Arguments string as given by user. */
+	/** Arguments string as given by user or "" for no argument. */
 	char *args;
 };