diff mbox

[dpdk-dev,1/3] lib/cmdline: add echo support in batch loading from file

Message ID 20171115154545.8936-2-xuemingl@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Xueming Li Nov. 15, 2017, 3:45 p.m. UTC 
  Add echo option to echo commandline to screen when running loaded
scripts from file.

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 lib/librte_cmdline/cmdline_socket.c | 5 +++--
 lib/librte_cmdline/cmdline_socket.h | 3 ++-
 2 files changed, 5 insertions(+), 3 deletions(-)

Comments

Burakov, Anatoly Dec. 11, 2017, 12:29 p.m. UTC | #1 
On 15-Nov-17 3:45 PM, Xueming Li wrote:
> Add echo option to echo commandline to screen when running loaded
> scripts from file.
> 
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---

<...snip...>

> @@ -86,7 +87,7 @@ cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path)
>   		dprintf("open() failed\n");
>   		return NULL;
>   	}
> -	return cmdline_new(ctx, prompt, fd, -1);
> +	return cmdline_new(ctx, prompt, fd, echo ? 1 : -1);

This should probably be "echo ? STDOUT_FILENO : -1", to make the 
intention clearer.

>
Olivier Matz Dec. 19, 2017, 10:30 a.m. UTC | #2 
Hi Xueming,

On Wed, Nov 15, 2017 at 11:45:43PM +0800, Xueming Li wrote:
> Add echo option to echo commandline to screen when running loaded
> scripts from file.
> 
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---
>  lib/librte_cmdline/cmdline_socket.c | 5 +++--
>  lib/librte_cmdline/cmdline_socket.h | 3 ++-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
> index 3fc243b70..e57ddeffb 100644
> --- a/lib/librte_cmdline/cmdline_socket.c
> +++ b/lib/librte_cmdline/cmdline_socket.c
> @@ -73,7 +73,8 @@
>  #include "cmdline.h"
>  
>  struct cmdline *
> -cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path)
> +cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path,
> +		 int echo)
>  {
>  	int fd;
>  
> @@ -86,7 +87,7 @@ cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path)
>  		dprintf("open() failed\n");
>  		return NULL;
>  	}
> -	return cmdline_new(ctx, prompt, fd, -1);
> +	return cmdline_new(ctx, prompt, fd, echo ? 1 : -1);
>  }
>  
>  struct cmdline *
> diff --git a/lib/librte_cmdline/cmdline_socket.h b/lib/librte_cmdline/cmdline_socket.h
> index aa6068e7e..208134b12 100644
> --- a/lib/librte_cmdline/cmdline_socket.h
> +++ b/lib/librte_cmdline/cmdline_socket.h
> @@ -68,7 +68,8 @@
>  extern "C" {
>  #endif
>  
> -struct cmdline *cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path);
> +struct cmdline *cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt,
> +				 const char *path, int echo);
>  struct cmdline *cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt);
>  void cmdline_stdin_exit(struct cmdline *cl);

This breaks the API and ABI.
And it also breaks the compilation, because the modifications of applications
are done in the next commits.

You can send a deprecation notice, and this patch could be added in for 18.05.

But instead, I suggest you to reimplement your own version of cmdline_file_new()
with the echo feature inside testpmd.
Xueming Li Dec. 19, 2017, 11:20 a.m. UTC | #3 
The idea of implementation in testpmd looks good, thanks!

Then I will combine all patches in v1 to avoid breaking build.

> -----Original Message-----

> From: Olivier MATZ [mailto:olivier.matz@6wind.com]

> Sent: Tuesday, December 19, 2017 6:31 PM

> To: Xueming(Steven) Li <xuemingl@mellanox.com>

> Cc: Jingjing Wu <jingjing.wu@intel.com>; dev@dpdk.org

> Subject: Re: [PATCH 1/3] lib/cmdline: add echo support in batch loading

> from file

> 

> Hi Xueming,

> 

> On Wed, Nov 15, 2017 at 11:45:43PM +0800, Xueming Li wrote:

> > Add echo option to echo commandline to screen when running loaded

> > scripts from file.

> >

> > Signed-off-by: Xueming Li <xuemingl@mellanox.com>

> > ---

> >  lib/librte_cmdline/cmdline_socket.c | 5 +++--

> > lib/librte_cmdline/cmdline_socket.h | 3 ++-

> >  2 files changed, 5 insertions(+), 3 deletions(-)

> >

> > diff --git a/lib/librte_cmdline/cmdline_socket.c

> > b/lib/librte_cmdline/cmdline_socket.c

> > index 3fc243b70..e57ddeffb 100644

> > --- a/lib/librte_cmdline/cmdline_socket.c

> > +++ b/lib/librte_cmdline/cmdline_socket.c

> > @@ -73,7 +73,8 @@

> >  #include "cmdline.h"

> >

> >  struct cmdline *

> > -cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const

> > char *path)

> > +cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const

> char *path,

> > +		 int echo)

> >  {

> >  	int fd;

> >

> > @@ -86,7 +87,7 @@ cmdline_file_new(cmdline_parse_ctx_t *ctx, const char

> *prompt, const char *path)

> >  		dprintf("open() failed\n");

> >  		return NULL;

> >  	}

> > -	return cmdline_new(ctx, prompt, fd, -1);

> > +	return cmdline_new(ctx, prompt, fd, echo ? 1 : -1);

> >  }

> >

> >  struct cmdline *

> > diff --git a/lib/librte_cmdline/cmdline_socket.h

> > b/lib/librte_cmdline/cmdline_socket.h

> > index aa6068e7e..208134b12 100644

> > --- a/lib/librte_cmdline/cmdline_socket.h

> > +++ b/lib/librte_cmdline/cmdline_socket.h

> > @@ -68,7 +68,8 @@

> >  extern "C" {

> >  #endif

> >

> > -struct cmdline *cmdline_file_new(cmdline_parse_ctx_t *ctx, const char

> > *prompt, const char *path);

> > +struct cmdline *cmdline_file_new(cmdline_parse_ctx_t *ctx, const char

> *prompt,

> > +				 const char *path, int echo);

> >  struct cmdline *cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const

> > char *prompt);  void cmdline_stdin_exit(struct cmdline *cl);

> 

> This breaks the API and ABI.

> And it also breaks the compilation, because the modifications of

> applications are done in the next commits.

> 

> You can send a deprecation notice, and this patch could be added in for

> 18.05.

> 

> But instead, I suggest you to reimplement your own version of

> cmdline_file_new() with the echo feature inside testpmd.
diff mbox

Patch

diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
index 3fc243b70..e57ddeffb 100644
--- a/lib/librte_cmdline/cmdline_socket.c
+++ b/lib/librte_cmdline/cmdline_socket.c
@@ -73,7 +73,8 @@ 
 #include "cmdline.h"
 
 struct cmdline *
-cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path)
+cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path,
+		 int echo)
 {
 	int fd;
 
@@ -86,7 +87,7 @@  cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path)
 		dprintf("open() failed\n");
 		return NULL;
 	}
-	return cmdline_new(ctx, prompt, fd, -1);
+	return cmdline_new(ctx, prompt, fd, echo ? 1 : -1);
 }
 
 struct cmdline *
diff --git a/lib/librte_cmdline/cmdline_socket.h b/lib/librte_cmdline/cmdline_socket.h
index aa6068e7e..208134b12 100644
--- a/lib/librte_cmdline/cmdline_socket.h
+++ b/lib/librte_cmdline/cmdline_socket.h
@@ -68,7 +68,8 @@ 
 extern "C" {
 #endif
 
-struct cmdline *cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt, const char *path);
+struct cmdline *cmdline_file_new(cmdline_parse_ctx_t *ctx, const char *prompt,
+				 const char *path, int echo);
 struct cmdline *cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt);
 void cmdline_stdin_exit(struct cmdline *cl);