diff mbox

[dpdk-dev] cmdline: Fix broken functionality in FreeBSD

Message ID 1416493033-13450-1-git-send-email-sergio.gonzalez.monroy@intel.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Sergio Gonzalez Monroy Nov. 20, 2014, 2:17 p.m. UTC 
Some features of the cmdline were broken in FreeBSD as a result of
termios not being compiled.

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
 lib/librte_cmdline/cmdline.h        |  2 --
 lib/librte_cmdline/cmdline_socket.c | 10 +---------
 2 files changed, 1 insertion(+), 11 deletions(-)

Comments

Neil Horman Nov. 20, 2014, 2:20 p.m. UTC | #1 
On Thu, Nov 20, 2014 at 02:17:13PM +0000, Sergio Gonzalez Monroy wrote:
> Some features of the cmdline were broken in FreeBSD as a result of
> termios not being compiled.
> 
> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

Not sure I understand the changelog above.  You're removing ifdefs below which
makes sense, but are you now assuming that BSD will be built with termios
support, or do you need to add some alternate dependency check during the
configuration of DPDK?
Neil

> ---
>  lib/librte_cmdline/cmdline.h        |  2 --
>  lib/librte_cmdline/cmdline_socket.c | 10 +---------
>  2 files changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/lib/librte_cmdline/cmdline.h b/lib/librte_cmdline/cmdline.h
> index 4c28d37..06ae086 100644
> --- a/lib/librte_cmdline/cmdline.h
> +++ b/lib/librte_cmdline/cmdline.h
> @@ -71,9 +71,7 @@ struct cmdline {
>  	cmdline_parse_ctx_t *ctx;
>  	struct rdline rdl;
>  	char prompt[RDLINE_PROMPT_SIZE];
> -#ifdef RTE_EXEC_ENV_LINUXAPP
>  	struct termios oldterm;
> -#endif
>  };
>  
>  struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
> diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
> index b51b537..6820b6d 100644
> --- a/lib/librte_cmdline/cmdline_socket.c
> +++ b/lib/librte_cmdline/cmdline_socket.c
> @@ -93,7 +93,6 @@ struct cmdline *
>  cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt)
>  {
>  	struct cmdline *cl;
> -#ifdef RTE_EXEC_ENV_LINUXAPP
>  	struct termios oldterm, term;
>  
>  	tcgetattr(0, &oldterm);
> @@ -101,14 +100,12 @@ cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt)
>  	term.c_lflag &= ~(ICANON | ECHO | ISIG);
>  	tcsetattr(0, TCSANOW, &term);
>  	setbuf(stdin, NULL);
> -#endif
>  
>  	cl = cmdline_new(ctx, prompt, 0, 1);
>  
> -#ifdef RTE_EXEC_ENV_LINUXAPP
>  	if (cl)
>  		memcpy(&cl->oldterm, &oldterm, sizeof(term));
> -#endif
> +
>  	return cl;
>  }
>  
> @@ -118,10 +115,5 @@ cmdline_stdin_exit(struct cmdline *cl)
>  	if (!cl)
>  		return;
>  
> -#ifdef RTE_EXEC_ENV_LINUXAPP
>  	tcsetattr(fileno(stdin), TCSANOW, &cl->oldterm);
> -#else
> -	/* silent the compiler */
> -	(void)cl;
> -#endif
>  }
> -- 
> 2.1.0
> 
>
Sergio Gonzalez Monroy Nov. 20, 2014, 4:42 p.m. UTC | #2 
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Thursday, November 20, 2014 2:21 PM
> On Thu, Nov 20, 2014 at 02:17:13PM +0000, Sergio Gonzalez Monroy wrote:
> > Some features of the cmdline were broken in FreeBSD as a result of
> > termios not being compiled.
> >
> > Signed-off-by: Sergio Gonzalez Monroy
> > <sergio.gonzalez.monroy@intel.com>
> 
> Not sure I understand the changelog above.  You're removing ifdefs below
> which makes sense, but are you now assuming that BSD will be built with
> termios support, or do you need to add some alternate dependency check
> during the configuration of DPDK?
> Neil
> 
Yes, I was assuming that BSD has termios support.
Is it not a fair assumption?

Sergio
Neil Horman Nov. 20, 2014, 5:03 p.m. UTC | #3 
On Thu, Nov 20, 2014 at 04:42:23PM +0000, Gonzalez Monroy, Sergio wrote:
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Thursday, November 20, 2014 2:21 PM
> > On Thu, Nov 20, 2014 at 02:17:13PM +0000, Sergio Gonzalez Monroy wrote:
> > > Some features of the cmdline were broken in FreeBSD as a result of
> > > termios not being compiled.
> > >
> > > Signed-off-by: Sergio Gonzalez Monroy
> > > <sergio.gonzalez.monroy@intel.com>
> > 
> > Not sure I understand the changelog above.  You're removing ifdefs below
> > which makes sense, but are you now assuming that BSD will be built with
> > termios support, or do you need to add some alternate dependency check
> > during the configuration of DPDK?
> > Neil
> > 
> Yes, I was assuming that BSD has termios support.
> Is it not a fair assumption?
> 
No, I think its a perfectly fair assumption.  I was just trying to understand
the history of the ifdefs there.  Sounds like it was a dumb idea to intiially
ifdef the termios stuff out way back when.

Acked-by: Neil Horman <nhorman@tuxdriver.com>

> Sergio
>
Bruce Richardson Nov. 20, 2014, 5:19 p.m. UTC | #4 
On Thu, Nov 20, 2014 at 12:03:40PM -0500, Neil Horman wrote:
> On Thu, Nov 20, 2014 at 04:42:23PM +0000, Gonzalez Monroy, Sergio wrote:
> > > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > > Sent: Thursday, November 20, 2014 2:21 PM
> > > On Thu, Nov 20, 2014 at 02:17:13PM +0000, Sergio Gonzalez Monroy wrote:
> > > > Some features of the cmdline were broken in FreeBSD as a result of
> > > > termios not being compiled.
> > > >
> > > > Signed-off-by: Sergio Gonzalez Monroy
> > > > <sergio.gonzalez.monroy@intel.com>
> > > 
> > > Not sure I understand the changelog above.  You're removing ifdefs below
> > > which makes sense, but are you now assuming that BSD will be built with
> > > termios support, or do you need to add some alternate dependency check
> > > during the configuration of DPDK?
> > > Neil
> > > 
> > Yes, I was assuming that BSD has termios support.
> > Is it not a fair assumption?
> > 
> No, I think its a perfectly fair assumption.  I was just trying to understand
> the history of the ifdefs there.  Sounds like it was a dumb idea to intiially
> ifdef the termios stuff out way back when.
> 

The ifdef probably dates from when there were just two versions of DPDK: linux
and baremetal. Guess which one didn't have the termios support :-)

/Bruce

> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> 
> > Sergio
> >
Neil Horman Nov. 20, 2014, 6:31 p.m. UTC | #5 
On Thu, Nov 20, 2014 at 05:19:19PM +0000, Bruce Richardson wrote:
> On Thu, Nov 20, 2014 at 12:03:40PM -0500, Neil Horman wrote:
> > On Thu, Nov 20, 2014 at 04:42:23PM +0000, Gonzalez Monroy, Sergio wrote:
> > > > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > > > Sent: Thursday, November 20, 2014 2:21 PM
> > > > On Thu, Nov 20, 2014 at 02:17:13PM +0000, Sergio Gonzalez Monroy wrote:
> > > > > Some features of the cmdline were broken in FreeBSD as a result of
> > > > > termios not being compiled.
> > > > >
> > > > > Signed-off-by: Sergio Gonzalez Monroy
> > > > > <sergio.gonzalez.monroy@intel.com>
> > > > 
> > > > Not sure I understand the changelog above.  You're removing ifdefs below
> > > > which makes sense, but are you now assuming that BSD will be built with
> > > > termios support, or do you need to add some alternate dependency check
> > > > during the configuration of DPDK?
> > > > Neil
> > > > 
> > > Yes, I was assuming that BSD has termios support.
> > > Is it not a fair assumption?
> > > 
> > No, I think its a perfectly fair assumption.  I was just trying to understand
> > the history of the ifdefs there.  Sounds like it was a dumb idea to intiially
> > ifdef the termios stuff out way back when.
> > 
> 
> The ifdef probably dates from when there were just two versions of DPDK: linux
> and baremetal. Guess which one didn't have the termios support :-)
> 
> /Bruce
> 
Ah, thanks for the history Bruce :)
Neil

> > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > 
> > > Sergio
> > > 
>
Bruce Richardson Nov. 21, 2014, 9:18 a.m. UTC | #6 
On Thu, Nov 20, 2014 at 02:17:13PM +0000, Sergio Gonzalez Monroy wrote:
> Some features of the cmdline were broken in FreeBSD as a result of
> termios not being compiled.
> 
> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_cmdline/cmdline.h        |  2 --
>  lib/librte_cmdline/cmdline_socket.c | 10 +---------
>  2 files changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/lib/librte_cmdline/cmdline.h b/lib/librte_cmdline/cmdline.h
> index 4c28d37..06ae086 100644
> --- a/lib/librte_cmdline/cmdline.h
> +++ b/lib/librte_cmdline/cmdline.h
> @@ -71,9 +71,7 @@ struct cmdline {
>  	cmdline_parse_ctx_t *ctx;
>  	struct rdline rdl;
>  	char prompt[RDLINE_PROMPT_SIZE];
> -#ifdef RTE_EXEC_ENV_LINUXAPP
>  	struct termios oldterm;
> -#endif
>  };
>  
>  struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
> diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
> index b51b537..6820b6d 100644
> --- a/lib/librte_cmdline/cmdline_socket.c
> +++ b/lib/librte_cmdline/cmdline_socket.c
> @@ -93,7 +93,6 @@ struct cmdline *
>  cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt)
>  {
>  	struct cmdline *cl;
> -#ifdef RTE_EXEC_ENV_LINUXAPP
>  	struct termios oldterm, term;
>  
>  	tcgetattr(0, &oldterm);
> @@ -101,14 +100,12 @@ cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt)
>  	term.c_lflag &= ~(ICANON | ECHO | ISIG);
>  	tcsetattr(0, TCSANOW, &term);
>  	setbuf(stdin, NULL);
> -#endif
>  
>  	cl = cmdline_new(ctx, prompt, 0, 1);
>  
> -#ifdef RTE_EXEC_ENV_LINUXAPP
>  	if (cl)
>  		memcpy(&cl->oldterm, &oldterm, sizeof(term));
> -#endif
> +
>  	return cl;
>  }
>  
> @@ -118,10 +115,5 @@ cmdline_stdin_exit(struct cmdline *cl)
>  	if (!cl)
>  		return;
>  
> -#ifdef RTE_EXEC_ENV_LINUXAPP
>  	tcsetattr(fileno(stdin), TCSANOW, &cl->oldterm);
> -#else
> -	/* silent the compiler */
> -	(void)cl;
> -#endif
>  }
> -- 
> 2.1.0
>
Thomas Monjalon Nov. 24, 2014, 3:51 p.m. UTC | #7 
> > Some features of the cmdline were broken in FreeBSD as a result of
> > termios not being compiled.
> > 
> > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Applied

Thanks
diff mbox

Patch

diff --git a/lib/librte_cmdline/cmdline.h b/lib/librte_cmdline/cmdline.h
index 4c28d37..06ae086 100644
--- a/lib/librte_cmdline/cmdline.h
+++ b/lib/librte_cmdline/cmdline.h
@@ -71,9 +71,7 @@  struct cmdline {
 	cmdline_parse_ctx_t *ctx;
 	struct rdline rdl;
 	char prompt[RDLINE_PROMPT_SIZE];
-#ifdef RTE_EXEC_ENV_LINUXAPP
 	struct termios oldterm;
-#endif
 };
 
 struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
diff --git a/lib/librte_cmdline/cmdline_socket.c b/lib/librte_cmdline/cmdline_socket.c
index b51b537..6820b6d 100644
--- a/lib/librte_cmdline/cmdline_socket.c
+++ b/lib/librte_cmdline/cmdline_socket.c
@@ -93,7 +93,6 @@  struct cmdline *
 cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt)
 {
 	struct cmdline *cl;
-#ifdef RTE_EXEC_ENV_LINUXAPP
 	struct termios oldterm, term;
 
 	tcgetattr(0, &oldterm);
@@ -101,14 +100,12 @@  cmdline_stdin_new(cmdline_parse_ctx_t *ctx, const char *prompt)
 	term.c_lflag &= ~(ICANON | ECHO | ISIG);
 	tcsetattr(0, TCSANOW, &term);
 	setbuf(stdin, NULL);
-#endif
 
 	cl = cmdline_new(ctx, prompt, 0, 1);
 
-#ifdef RTE_EXEC_ENV_LINUXAPP
 	if (cl)
 		memcpy(&cl->oldterm, &oldterm, sizeof(term));
-#endif
+
 	return cl;
 }
 
@@ -118,10 +115,5 @@  cmdline_stdin_exit(struct cmdline *cl)
 	if (!cl)
 		return;
 
-#ifdef RTE_EXEC_ENV_LINUXAPP
 	tcsetattr(fileno(stdin), TCSANOW, &cl->oldterm);
-#else
-	/* silent the compiler */
-	(void)cl;
-#endif
 }