[dpdk-dev] [PATCH] cmdline: make cmdline structure opaque

[dpdk-dev] [PATCH] cmdline: make cmdline structure opaque

[Dmitry Kozlyuk]

Remove the definition of `struct cmdline` from public header.
Deprecation notice:
https://mails.dpdk.org/archives/dev/2020-September/183310.html

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
Unfortunately there's no deprection notice for struct rdline.
If its definition was also hidden, the line buffer size limit
could be changed or removed completely in the future.

 lib/cmdline/cmdline.h         | 19 -------------------
 lib/cmdline/cmdline_private.h | 11 ++++++++++-
 2 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/lib/cmdline/cmdline.h b/lib/cmdline/cmdline.h
index c29762ddae..96674dfda2 100644
--- a/lib/cmdline/cmdline.h
+++ b/lib/cmdline/cmdline.h
@@ -7,10 +7,6 @@
 #ifndef _CMDLINE_H_
 #define _CMDLINE_H_
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-#include <termios.h>
-#endif
-
 #include <rte_common.h>
 #include <rte_compat.h>
 
@@ -27,23 +23,8 @@
 extern "C" {
 #endif
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-
-struct cmdline {
-	int s_in;
-	int s_out;
-	cmdline_parse_ctx_t *ctx;
-	struct rdline rdl;
-	char prompt[RDLINE_PROMPT_SIZE];
-	struct termios oldterm;
-};
-
-#else
-
 struct cmdline;
 
-#endif /* RTE_EXEC_ENV_WINDOWS */
-
 struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
 void cmdline_set_prompt(struct cmdline *cl, const char *prompt);
 void cmdline_free(struct cmdline *cl);
diff --git a/lib/cmdline/cmdline_private.h b/lib/cmdline/cmdline_private.h
index a87c45275c..7743807d0b 100644
--- a/lib/cmdline/cmdline_private.h
+++ b/lib/cmdline/cmdline_private.h
@@ -33,7 +33,16 @@ struct cmdline {
 	char repeated_char;
 	WORD repeat_count;
 };
-#endif
+#else
+struct cmdline {
+	int s_in;
+	int s_out;
+	cmdline_parse_ctx_t *ctx;
+	struct rdline rdl;
+	char prompt[RDLINE_PROMPT_SIZE];
+	struct termios oldterm;
+};
+#endif /* !RTE_EXEC_ENV_WINDOWS */
 
 /* Disable buffering and echoing, save previous settings to oldterm. */
 void terminal_adjust(struct cmdline *cl);
-- 
2.29.3

[dpdk-dev] [PATCH] cmdline: reduce ABI

[Dmitry Kozlyuk]

Remove the definition of `struct cmdline` from public header.
Deprecation notice:
https://mails.dpdk.org/archives/dev/2020-September/183310.html

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
I would also hide struct rdline to be able to alter buffer size,
but we don't have a deprecation notice for it.

 doc/guides/rel_notes/deprecation.rst   |  4 ----
 doc/guides/rel_notes/release_21_11.rst |  2 ++
 lib/cmdline/cmdline.h                  | 19 -------------------
 lib/cmdline/cmdline_private.h          |  8 +++++++-
 4 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 76a4abfd6b..a404276fa2 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -275,10 +275,6 @@ Deprecation Notices
 * metrics: The function ``rte_metrics_init`` will have a non-void return
   in order to notify errors instead of calling ``rte_exit``.
 
-* cmdline: ``cmdline`` structure will be made opaque to hide platform-specific
-  content. On Linux and FreeBSD, supported prior to DPDK 20.11,
-  original structure will be kept until DPDK 21.11.
-
 * security: The functions ``rte_security_set_pkt_metadata`` and
   ``rte_security_get_userdata`` will be made inline functions and additional
   flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index 675b573834..be73d17ef6 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -91,6 +91,8 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* cmdline: Made ``cmdline`` structure definition hidden on Linux and FreeBSD.
+
 
 ABI Changes
 -----------
diff --git a/lib/cmdline/cmdline.h b/lib/cmdline/cmdline.h
index c29762ddae..96674dfda2 100644
--- a/lib/cmdline/cmdline.h
+++ b/lib/cmdline/cmdline.h
@@ -7,10 +7,6 @@
 #ifndef _CMDLINE_H_
 #define _CMDLINE_H_
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-#include <termios.h>
-#endif
-
 #include <rte_common.h>
 #include <rte_compat.h>
 
@@ -27,23 +23,8 @@
 extern "C" {
 #endif
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-
-struct cmdline {
-	int s_in;
-	int s_out;
-	cmdline_parse_ctx_t *ctx;
-	struct rdline rdl;
-	char prompt[RDLINE_PROMPT_SIZE];
-	struct termios oldterm;
-};
-
-#else
-
 struct cmdline;
 
-#endif /* RTE_EXEC_ENV_WINDOWS */
-
 struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
 void cmdline_set_prompt(struct cmdline *cl, const char *prompt);
 void cmdline_free(struct cmdline *cl);
diff --git a/lib/cmdline/cmdline_private.h b/lib/cmdline/cmdline_private.h
index a87c45275c..2e93674c66 100644
--- a/lib/cmdline/cmdline_private.h
+++ b/lib/cmdline/cmdline_private.h
@@ -11,6 +11,8 @@
 #include <rte_os_shim.h>
 #ifdef RTE_EXEC_ENV_WINDOWS
 #include <rte_windows.h>
+#else
+#include <termios.h>
 #endif
 
 #include <cmdline.h>
@@ -22,6 +24,7 @@ struct terminal {
 	int is_console_input;
 	int is_console_output;
 };
+#endif
 
 struct cmdline {
 	int s_in;
@@ -29,11 +32,14 @@ struct cmdline {
 	cmdline_parse_ctx_t *ctx;
 	struct rdline rdl;
 	char prompt[RDLINE_PROMPT_SIZE];
+#ifdef RTE_EXEC_ENV_WINDOWS
 	struct terminal oldterm;
 	char repeated_char;
 	WORD repeat_count;
-};
+#else
+	struct termios oldterm;
 #endif
+};
 
 /* Disable buffering and echoing, save previous settings to oldterm. */
 void terminal_adjust(struct cmdline *cl);
-- 
2.29.3

Re: [dpdk-dev] [PATCH] cmdline: reduce ABI

[David Marchand]

On Sat, Sep 11, 2021 at 1:17 AM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> Remove the definition of `struct cmdline` from public header.
> Deprecation notice:
> https://mails.dpdk.org/archives/dev/2020-September/183310.html
>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

This patch lgtm.
Acked-by: David Marchand <david.marchand@redhat.com>


> ---
> I would also hide struct rdline to be able to alter buffer size,
> but we don't have a deprecation notice for it.

Fyi, I found one project looking into a rdline pointer to get the back
reference to cmdline stored in opaque.
https://github.com/Gandi/packet-journey/blob/master/app/cmdline.c#L1398

This cmdline pointer is then dereferenced to get s_out.
Given that we announced cmdline becoming opaque, they would have to
handle the first API change in any case.
I don't think another API change would really make a big difference to them.

Plus, this project seems stuck to 18.08 support.



--
David Marchand

Re: [dpdk-dev] [PATCH] cmdline: reduce ABI

[Olivier Matz]

Hi Dmitry,

On Mon, Sep 20, 2021 at 01:11:23PM +0200, David Marchand wrote:
> On Sat, Sep 11, 2021 at 1:17 AM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
> >
> > Remove the definition of `struct cmdline` from public header.
> > Deprecation notice:
> > https://mails.dpdk.org/archives/dev/2020-September/183310.html
> >
> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> 
> This patch lgtm.
> Acked-by: David Marchand <david.marchand@redhat.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Many thanks Dmitry for taking care of this.


> > ---
> > I would also hide struct rdline to be able to alter buffer size,
> > but we don't have a deprecation notice for it.
> 
> Fyi, I found one project looking into a rdline pointer to get the back
> reference to cmdline stored in opaque.
> https://github.com/Gandi/packet-journey/blob/master/app/cmdline.c#L1398
> 
> This cmdline pointer is then dereferenced to get s_out.
> Given that we announced cmdline becoming opaque, they would have to
> handle the first API change in any case.
> I don't think another API change would really make a big difference to them.
> 
> Plus, this project seems stuck to 18.08 support.

I agree with you and David, it would make sense to also hide the rdline
struct at the same time.


Olivier

[dpdk-dev] [PATCH v2 0/2] cmdline: reduce ABI

[Dmitry Kozlyuk]

Hide struct cmdline following the deprecation notice.
Hide struct rdline following the v1 discussion.

Dmitry Kozlyuk (2):
  cmdline: make struct cmdline opaque
  cmdline: make struct rdline opaque

 app/test-cmdline/commands.c            |  2 +-
 app/test/test_cmdline_lib.c            | 19 +++---
 doc/guides/rel_notes/deprecation.rst   |  4 --
 doc/guides/rel_notes/release_21_11.rst |  2 +
 lib/cmdline/cmdline.c                  |  3 +-
 lib/cmdline/cmdline.h                  | 19 ------
 lib/cmdline/cmdline_cirbuf.c           |  1 -
 lib/cmdline/cmdline_private.h          | 57 +++++++++++++++++-
 lib/cmdline/cmdline_rdline.c           | 50 +++++++++++++++-
 lib/cmdline/cmdline_rdline.h           | 83 +++++++++-----------------
 lib/cmdline/version.map                |  8 ++-
 11 files changed, 155 insertions(+), 93 deletions(-)

-- 
2.29.3

[dpdk-dev] [PATCH v2 1/2] cmdline: make struct cmdline opaque

[Dmitry Kozlyuk]

Remove the definition of `struct cmdline` from public header.
Deprecation notice:
https://mails.dpdk.org/archives/dev/2020-September/183310.html

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: David Marchand <david.marchand@redhat.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
---
 doc/guides/rel_notes/deprecation.rst   |  4 ----
 doc/guides/rel_notes/release_21_11.rst |  2 ++
 lib/cmdline/cmdline.h                  | 19 -------------------
 lib/cmdline/cmdline_private.h          |  8 +++++++-
 4 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 76a4abfd6b..a404276fa2 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -275,10 +275,6 @@ Deprecation Notices
 * metrics: The function ``rte_metrics_init`` will have a non-void return
   in order to notify errors instead of calling ``rte_exit``.
 
-* cmdline: ``cmdline`` structure will be made opaque to hide platform-specific
-  content. On Linux and FreeBSD, supported prior to DPDK 20.11,
-  original structure will be kept until DPDK 21.11.
-
 * security: The functions ``rte_security_set_pkt_metadata`` and
   ``rte_security_get_userdata`` will be made inline functions and additional
   flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index b55900936d..18377e5813 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -101,6 +101,8 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* cmdline: Made ``cmdline`` structure definition hidden on Linux and FreeBSD.
+
 
 ABI Changes
 -----------
diff --git a/lib/cmdline/cmdline.h b/lib/cmdline/cmdline.h
index c29762ddae..96674dfda2 100644
--- a/lib/cmdline/cmdline.h
+++ b/lib/cmdline/cmdline.h
@@ -7,10 +7,6 @@
 #ifndef _CMDLINE_H_
 #define _CMDLINE_H_
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-#include <termios.h>
-#endif
-
 #include <rte_common.h>
 #include <rte_compat.h>
 
@@ -27,23 +23,8 @@
 extern "C" {
 #endif
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-
-struct cmdline {
-	int s_in;
-	int s_out;
-	cmdline_parse_ctx_t *ctx;
-	struct rdline rdl;
-	char prompt[RDLINE_PROMPT_SIZE];
-	struct termios oldterm;
-};
-
-#else
-
 struct cmdline;
 
-#endif /* RTE_EXEC_ENV_WINDOWS */
-
 struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
 void cmdline_set_prompt(struct cmdline *cl, const char *prompt);
 void cmdline_free(struct cmdline *cl);
diff --git a/lib/cmdline/cmdline_private.h b/lib/cmdline/cmdline_private.h
index a87c45275c..2e93674c66 100644
--- a/lib/cmdline/cmdline_private.h
+++ b/lib/cmdline/cmdline_private.h
@@ -11,6 +11,8 @@
 #include <rte_os_shim.h>
 #ifdef RTE_EXEC_ENV_WINDOWS
 #include <rte_windows.h>
+#else
+#include <termios.h>
 #endif
 
 #include <cmdline.h>
@@ -22,6 +24,7 @@ struct terminal {
 	int is_console_input;
 	int is_console_output;
 };
+#endif
 
 struct cmdline {
 	int s_in;
@@ -29,11 +32,14 @@ struct cmdline {
 	cmdline_parse_ctx_t *ctx;
 	struct rdline rdl;
 	char prompt[RDLINE_PROMPT_SIZE];
+#ifdef RTE_EXEC_ENV_WINDOWS
 	struct terminal oldterm;
 	char repeated_char;
 	WORD repeat_count;
-};
+#else
+	struct termios oldterm;
 #endif
+};
 
 /* Disable buffering and echoing, save previous settings to oldterm. */
 void terminal_adjust(struct cmdline *cl);
-- 
2.29.3

[dpdk-dev] [PATCH v2 2/2] cmdline: make struct rdline opaque

[Dmitry Kozlyuk]

Hide struct rdline definition and some RDLINE_* constants in order
to be able to change internal buffer sizes transparently to the user.
Add new functions:

* rdline_create(): allocate and initialize struct rdline.
  This function replaces rdline_init() and takes an extra parameter:
  opaque user data for the callbacks.
* rdline_free(): deallocate struct rdline.
* rdline_get_history_buffer_size(): for use in tests.
* rdline_get_opaque(): to obtain user data in callback functions.

Remove rdline_init() function from library headers and export list,
because using it requires the knowledge of sizeof(struct rdline).

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 app/test-cmdline/commands.c   |  2 +-
 app/test/test_cmdline_lib.c   | 19 +++++---
 lib/cmdline/cmdline.c         |  3 +-
 lib/cmdline/cmdline_cirbuf.c  |  1 -
 lib/cmdline/cmdline_private.h | 49 +++++++++++++++++++++
 lib/cmdline/cmdline_rdline.c  | 50 +++++++++++++++++++--
 lib/cmdline/cmdline_rdline.h  | 83 ++++++++++++-----------------------
 lib/cmdline/version.map       |  8 +++-
 8 files changed, 146 insertions(+), 69 deletions(-)

diff --git a/app/test-cmdline/commands.c b/app/test-cmdline/commands.c
index d732976f08..a13e1d1afd 100644
--- a/app/test-cmdline/commands.c
+++ b/app/test-cmdline/commands.c
@@ -297,7 +297,7 @@ cmd_get_history_bufsize_parsed(__rte_unused void *parsed_result,
 	struct rdline *rdl = cmdline_get_rdline(cl);
 
 	cmdline_printf(cl, "History buffer size: %zu\n",
-			sizeof(rdl->history_buf));
+			rdline_get_history_buffer_size(rdl));
 }
 
 cmdline_parse_token_string_t cmd_get_history_bufsize_tok =
diff --git a/app/test/test_cmdline_lib.c b/app/test/test_cmdline_lib.c
index d5a09b4541..367dcad5be 100644
--- a/app/test/test_cmdline_lib.c
+++ b/app/test/test_cmdline_lib.c
@@ -83,18 +83,18 @@ test_cmdline_parse_fns(void)
 static int
 test_cmdline_rdline_fns(void)
 {
-	struct rdline rdl;
+	struct rdline *rdl = NULL;
 	rdline_write_char_t *wc = &cmdline_write_char;
 	rdline_validate_t *v = &valid_buffer;
 	rdline_complete_t *c = &complete_buffer;
 
-	if (rdline_init(NULL, wc, v, c) >= 0)
+	if (rdline_create(NULL, wc, v, c, NULL) >= 0)
 		goto error;
-	if (rdline_init(&rdl, NULL, v, c) >= 0)
+	if (rdline_create(&rdl, NULL, v, c, NULL) >= 0)
 		goto error;
-	if (rdline_init(&rdl, wc, NULL, c) >= 0)
+	if (rdline_create(&rdl, wc, NULL, c, NULL) >= 0)
 		goto error;
-	if (rdline_init(&rdl, wc, v, NULL) >= 0)
+	if (rdline_create(&rdl, wc, v, NULL, NULL) >= 0)
 		goto error;
 	if (rdline_char_in(NULL, 0) >= 0)
 		goto error;
@@ -102,25 +102,30 @@ test_cmdline_rdline_fns(void)
 		goto error;
 	if (rdline_add_history(NULL, "history") >= 0)
 		goto error;
-	if (rdline_add_history(&rdl, NULL) >= 0)
+	if (rdline_add_history(rdl, NULL) >= 0)
 		goto error;
 	if (rdline_get_history_item(NULL, 0) != NULL)
 		goto error;
 
 	/* void functions */
+	rdline_get_history_buffer_size(NULL);
+	rdline_get_opaque(NULL);
 	rdline_newline(NULL, "prompt");
-	rdline_newline(&rdl, NULL);
+	rdline_newline(rdl, NULL);
 	rdline_stop(NULL);
 	rdline_quit(NULL);
 	rdline_restart(NULL);
 	rdline_redisplay(NULL);
 	rdline_reset(NULL);
 	rdline_clear_history(NULL);
+	rdline_free(NULL);
 
+	rdline_free(rdl);
 	return 0;
 
 error:
 	printf("Error: function accepted null parameter!\n");
+	rdline_free(rdl);
 	return -1;
 }
 
diff --git a/lib/cmdline/cmdline.c b/lib/cmdline/cmdline.c
index a176d15130..8f1854cb0b 100644
--- a/lib/cmdline/cmdline.c
+++ b/lib/cmdline/cmdline.c
@@ -85,13 +85,12 @@ cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out)
 	cl->ctx = ctx;
 
 	ret = rdline_init(&cl->rdl, cmdline_write_char, cmdline_valid_buffer,
-			cmdline_complete_buffer);
+			cmdline_complete_buffer, cl);
 	if (ret != 0) {
 		free(cl);
 		return NULL;
 	}
 
-	cl->rdl.opaque = cl;
 	cmdline_set_prompt(cl, prompt);
 	rdline_newline(&cl->rdl, cl->prompt);
 
diff --git a/lib/cmdline/cmdline_cirbuf.c b/lib/cmdline/cmdline_cirbuf.c
index 829a8af563..cbb76a7016 100644
--- a/lib/cmdline/cmdline_cirbuf.c
+++ b/lib/cmdline/cmdline_cirbuf.c
@@ -10,7 +10,6 @@
 
 #include "cmdline_cirbuf.h"
 
-
 int
 cirbuf_init(struct cirbuf *cbuf, char *buf, unsigned int start, unsigned int maxlen)
 {
diff --git a/lib/cmdline/cmdline_private.h b/lib/cmdline/cmdline_private.h
index 2e93674c66..c2e906d8de 100644
--- a/lib/cmdline/cmdline_private.h
+++ b/lib/cmdline/cmdline_private.h
@@ -17,6 +17,49 @@
 
 #include <cmdline.h>
 
+#define RDLINE_BUF_SIZE 512
+#define RDLINE_PROMPT_SIZE  32
+#define RDLINE_VT100_BUF_SIZE  8
+#define RDLINE_HISTORY_BUF_SIZE BUFSIZ
+#define RDLINE_HISTORY_MAX_LINE 64
+
+enum rdline_status {
+	RDLINE_INIT,
+	RDLINE_RUNNING,
+	RDLINE_EXITED
+};
+
+struct rdline {
+	enum rdline_status status;
+	/* rdline bufs */
+	struct cirbuf left;
+	struct cirbuf right;
+	char left_buf[RDLINE_BUF_SIZE+2]; /* reserve 2 chars for the \n\0 */
+	char right_buf[RDLINE_BUF_SIZE];
+
+	char prompt[RDLINE_PROMPT_SIZE];
+	unsigned int prompt_size;
+
+	char kill_buf[RDLINE_BUF_SIZE];
+	unsigned int kill_size;
+
+	/* history */
+	struct cirbuf history;
+	char history_buf[RDLINE_HISTORY_BUF_SIZE];
+	int history_cur_line;
+
+	/* callbacks and func pointers */
+	rdline_write_char_t *write_char;
+	rdline_validate_t *validate;
+	rdline_complete_t *complete;
+
+	/* vt100 parser */
+	struct cmdline_vt100 vt100;
+
+	/* opaque pointer */
+	void *opaque;
+};
+
 #ifdef RTE_EXEC_ENV_WINDOWS
 struct terminal {
 	DWORD input_mode;
@@ -57,4 +100,10 @@ ssize_t cmdline_read_char(struct cmdline *cl, char *c);
 __rte_format_printf(2, 0)
 int cmdline_vdprintf(int fd, const char *format, va_list op);
 
+int rdline_init(struct rdline *rdl,
+		rdline_write_char_t *write_char,
+		rdline_validate_t *validate,
+		rdline_complete_t *complete,
+		void *opaque);
+
 #endif
diff --git a/lib/cmdline/cmdline_rdline.c b/lib/cmdline/cmdline_rdline.c
index 2cb53e38f2..a525513465 100644
--- a/lib/cmdline/cmdline_rdline.c
+++ b/lib/cmdline/cmdline_rdline.c
@@ -13,6 +13,7 @@
 #include <ctype.h>
 
 #include "cmdline_cirbuf.h"
+#include "cmdline_private.h"
 #include "cmdline_rdline.h"
 
 static void rdline_puts(struct rdline *rdl, const char *buf);
@@ -37,9 +38,10 @@ isblank2(char c)
 
 int
 rdline_init(struct rdline *rdl,
-		 rdline_write_char_t *write_char,
-		 rdline_validate_t *validate,
-		 rdline_complete_t *complete)
+	    rdline_write_char_t *write_char,
+	    rdline_validate_t *validate,
+	    rdline_complete_t *complete,
+	    void *opaque)
 {
 	if (!rdl || !write_char || !validate || !complete)
 		return -EINVAL;
@@ -47,10 +49,40 @@ rdline_init(struct rdline *rdl,
 	rdl->validate = validate;
 	rdl->complete = complete;
 	rdl->write_char = write_char;
+	rdl->opaque = opaque;
 	rdl->status = RDLINE_INIT;
 	return cirbuf_init(&rdl->history, rdl->history_buf, 0, RDLINE_HISTORY_BUF_SIZE);
 }
 
+int
+rdline_create(struct rdline **out,
+	      rdline_write_char_t *write_char,
+	      rdline_validate_t *validate,
+	      rdline_complete_t *complete,
+	      void *opaque)
+{
+	struct rdline *rdl;
+	int ret;
+
+	if (out == NULL)
+		return -EINVAL;
+	rdl = malloc(sizeof(*rdl));
+	if (rdl == NULL)
+		return -ENOMEM;
+	ret = rdline_init(rdl, write_char, validate, complete, opaque);
+	if (ret < 0)
+		free(rdl);
+	else
+		*out = rdl;
+	return ret;
+}
+
+void
+rdline_free(struct rdline *rdl)
+{
+	free(rdl);
+}
+
 void
 rdline_newline(struct rdline *rdl, const char *prompt)
 {
@@ -564,6 +596,18 @@ rdline_get_history_item(struct rdline * rdl, unsigned int idx)
 	return NULL;
 }
 
+size_t
+rdline_get_history_buffer_size(struct rdline *rdl)
+{
+	return sizeof(rdl->history_buf);
+}
+
+void *
+rdline_get_opaque(struct rdline *rdl)
+{
+	return rdl != NULL ? rdl->opaque : NULL;
+}
+
 int
 rdline_add_history(struct rdline * rdl, const char * buf)
 {
diff --git a/lib/cmdline/cmdline_rdline.h b/lib/cmdline/cmdline_rdline.h
index d2170293de..df69c7992f 100644
--- a/lib/cmdline/cmdline_rdline.h
+++ b/lib/cmdline/cmdline_rdline.h
@@ -10,9 +10,7 @@
 /**
  * This file is a small equivalent to the GNU readline library, but it
  * was originally designed for small systems, like Atmel AVR
- * microcontrollers (8 bits). Indeed, we don't use any malloc that is
- * sometimes not implemented (or just not recommended) on such
- * systems.
+ * microcontrollers (8 bits). It only uses malloc() on object creation.
  *
  * Obviously, it does not support as many things as the GNU readline,
  * but at least it supports some interesting features like a kill
@@ -38,19 +36,6 @@
 extern "C" {
 #endif
 
-/* configuration */
-#define RDLINE_BUF_SIZE 512
-#define RDLINE_PROMPT_SIZE  32
-#define RDLINE_VT100_BUF_SIZE  8
-#define RDLINE_HISTORY_BUF_SIZE BUFSIZ
-#define RDLINE_HISTORY_MAX_LINE 64
-
-enum rdline_status {
-	RDLINE_INIT,
-	RDLINE_RUNNING,
-	RDLINE_EXITED
-};
-
 struct rdline;
 
 typedef int (rdline_write_char_t)(struct rdline *rdl, char);
@@ -60,52 +45,32 @@ typedef int (rdline_complete_t)(struct rdline *rdl, const char *buf,
 				char *dstbuf, unsigned int dstsize,
 				int *state);
 
-struct rdline {
-	enum rdline_status status;
-	/* rdline bufs */
-	struct cirbuf left;
-	struct cirbuf right;
-	char left_buf[RDLINE_BUF_SIZE+2]; /* reserve 2 chars for the \n\0 */
-	char right_buf[RDLINE_BUF_SIZE];
-
-	char prompt[RDLINE_PROMPT_SIZE];
-	unsigned int prompt_size;
-
-	char kill_buf[RDLINE_BUF_SIZE];
-	unsigned int kill_size;
-
-	/* history */
-	struct cirbuf history;
-	char history_buf[RDLINE_HISTORY_BUF_SIZE];
-	int history_cur_line;
-
-	/* callbacks and func pointers */
-	rdline_write_char_t *write_char;
-	rdline_validate_t *validate;
-	rdline_complete_t *complete;
-
-	/* vt100 parser */
-	struct cmdline_vt100 vt100;
-
-	/* opaque pointer */
-	void *opaque;
-};
-
 /**
- * Init fields for a struct rdline. Call this only once at the beginning
- * of your program.
- * \param rdl A pointer to an uninitialized struct rdline
+ * Allocate and initialize a new rdline instance.
+ *
+ * \param rdl Receives a pointer to the allocated structure.
  * \param write_char The function used by the function to write a character
  * \param validate A pointer to the function to execute when the
  *                 user validates the buffer.
  * \param complete A pointer to the function to execute when the
  *                 user completes the buffer.
+ * \param opaque User data for use in the callbacks.
+ *
+ * \return 0 on success, negative errno-style code in failure.
  */
-int rdline_init(struct rdline *rdl,
-		 rdline_write_char_t *write_char,
-		 rdline_validate_t *validate,
-		 rdline_complete_t *complete);
+int rdline_create(struct rdline **rdl,
+		  rdline_write_char_t *write_char,
+		  rdline_validate_t *validate,
+		  rdline_complete_t *complete,
+		  void *opaque);
 
+/**
+ * Free an rdline instance.
+ *
+ * \param rdl A pointer to an initialized struct rdline.
+ *            If NULL, this function is a no-op.
+ */
+void rdline_free(struct rdline *rdl);
 
 /**
  * Init the current buffer, and display a prompt.
@@ -194,6 +159,16 @@ void rdline_clear_history(struct rdline *rdl);
  */
 char *rdline_get_history_item(struct rdline *rdl, unsigned int i);
 
+/**
+ * Get maximum history buffer size.
+ */
+size_t rdline_get_history_buffer_size(struct rdline *rdl);
+
+/**
+ * Get the opaque pointer supplied on struct rdline creation.
+ */
+void *rdline_get_opaque(struct rdline *rdl);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/cmdline/version.map b/lib/cmdline/version.map
index 980adb4f23..13feb4d0a5 100644
--- a/lib/cmdline/version.map
+++ b/lib/cmdline/version.map
@@ -57,7 +57,6 @@ DPDK_22 {
 	rdline_clear_history;
 	rdline_get_buffer;
 	rdline_get_history_item;
-	rdline_init;
 	rdline_newline;
 	rdline_quit;
 	rdline_redisplay;
@@ -73,7 +72,14 @@ DPDK_22 {
 EXPERIMENTAL {
 	global:
 
+	# added in 20.11
 	cmdline_get_rdline;
 
+	# added in 21.11
+	rdline_create;
+	rdline_free;
+	rdline_get_history_buffer_size;
+	rdline_get_opaque;
+
 	local: *;
 };
-- 
2.29.3

[dpdk-dev] [PATCH v3 0/2] cmdline: reduce ABI

[Dmitry Kozlyuk]

Hide struct cmdline following the deprecation notice.
Hide struct rdline following the v1 discussion.

v3: add experimental tags and releae notes for rdline.
v2: also hide struct rdline (David, Olivier).

Dmitry Kozlyuk (2):
  cmdline: make struct cmdline opaque
  cmdline: make struct rdline opaque

 app/test-cmdline/commands.c            |  2 +-
 app/test/test_cmdline_lib.c            | 19 ++++--
 doc/guides/rel_notes/deprecation.rst   |  4 --
 doc/guides/rel_notes/release_21_11.rst |  5 ++
 lib/cmdline/cmdline.c                  |  3 +-
 lib/cmdline/cmdline.h                  | 19 ------
 lib/cmdline/cmdline_cirbuf.c           |  1 -
 lib/cmdline/cmdline_private.h          | 57 ++++++++++++++++-
 lib/cmdline/cmdline_rdline.c           | 50 ++++++++++++++-
 lib/cmdline/cmdline_rdline.h           | 88 ++++++++++----------------
 lib/cmdline/version.map                |  8 ++-
 11 files changed, 163 insertions(+), 93 deletions(-)

-- 
2.29.3

[dpdk-dev] [PATCH v3 1/2] cmdline: make struct cmdline opaque

[Dmitry Kozlyuk]

Remove the definition of `struct cmdline` from public header.
Deprecation notice:
https://mails.dpdk.org/archives/dev/2020-September/183310.html

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: David Marchand <david.marchand@redhat.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
---
 doc/guides/rel_notes/deprecation.rst   |  4 ----
 doc/guides/rel_notes/release_21_11.rst |  2 ++
 lib/cmdline/cmdline.h                  | 19 -------------------
 lib/cmdline/cmdline_private.h          |  8 +++++++-
 4 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 76a4abfd6b..a404276fa2 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -275,10 +275,6 @@ Deprecation Notices
 * metrics: The function ``rte_metrics_init`` will have a non-void return
   in order to notify errors instead of calling ``rte_exit``.
 
-* cmdline: ``cmdline`` structure will be made opaque to hide platform-specific
-  content. On Linux and FreeBSD, supported prior to DPDK 20.11,
-  original structure will be kept until DPDK 21.11.
-
 * security: The functions ``rte_security_set_pkt_metadata`` and
   ``rte_security_get_userdata`` will be made inline functions and additional
   flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index b55900936d..18377e5813 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -101,6 +101,8 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* cmdline: Made ``cmdline`` structure definition hidden on Linux and FreeBSD.
+
 
 ABI Changes
 -----------
diff --git a/lib/cmdline/cmdline.h b/lib/cmdline/cmdline.h
index c29762ddae..96674dfda2 100644
--- a/lib/cmdline/cmdline.h
+++ b/lib/cmdline/cmdline.h
@@ -7,10 +7,6 @@
 #ifndef _CMDLINE_H_
 #define _CMDLINE_H_
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-#include <termios.h>
-#endif
-
 #include <rte_common.h>
 #include <rte_compat.h>
 
@@ -27,23 +23,8 @@
 extern "C" {
 #endif
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-
-struct cmdline {
-	int s_in;
-	int s_out;
-	cmdline_parse_ctx_t *ctx;
-	struct rdline rdl;
-	char prompt[RDLINE_PROMPT_SIZE];
-	struct termios oldterm;
-};
-
-#else
-
 struct cmdline;
 
-#endif /* RTE_EXEC_ENV_WINDOWS */
-
 struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
 void cmdline_set_prompt(struct cmdline *cl, const char *prompt);
 void cmdline_free(struct cmdline *cl);
diff --git a/lib/cmdline/cmdline_private.h b/lib/cmdline/cmdline_private.h
index a87c45275c..2e93674c66 100644
--- a/lib/cmdline/cmdline_private.h
+++ b/lib/cmdline/cmdline_private.h
@@ -11,6 +11,8 @@
 #include <rte_os_shim.h>
 #ifdef RTE_EXEC_ENV_WINDOWS
 #include <rte_windows.h>
+#else
+#include <termios.h>
 #endif
 
 #include <cmdline.h>
@@ -22,6 +24,7 @@ struct terminal {
 	int is_console_input;
 	int is_console_output;
 };
+#endif
 
 struct cmdline {
 	int s_in;
@@ -29,11 +32,14 @@ struct cmdline {
 	cmdline_parse_ctx_t *ctx;
 	struct rdline rdl;
 	char prompt[RDLINE_PROMPT_SIZE];
+#ifdef RTE_EXEC_ENV_WINDOWS
 	struct terminal oldterm;
 	char repeated_char;
 	WORD repeat_count;
-};
+#else
+	struct termios oldterm;
 #endif
+};
 
 /* Disable buffering and echoing, save previous settings to oldterm. */
 void terminal_adjust(struct cmdline *cl);
-- 
2.29.3

[dpdk-dev] [PATCH v3 2/2] cmdline: make struct rdline opaque

[Dmitry Kozlyuk]

Hide struct rdline definition and some RDLINE_* constants in order
to be able to change internal buffer sizes transparently to the user.
Add new functions:

* rdline_create(): allocate and initialize struct rdline.
  This function replaces rdline_init() and takes an extra parameter:
  opaque user data for the callbacks.
* rdline_free(): deallocate struct rdline.
* rdline_get_history_buffer_size(): for use in tests.
* rdline_get_opaque(): to obtain user data in callback functions.

Remove rdline_init() function from library headers and export list,
because using it requires the knowledge of sizeof(struct rdline).

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 app/test-cmdline/commands.c            |  2 +-
 app/test/test_cmdline_lib.c            | 19 ++++--
 doc/guides/rel_notes/release_21_11.rst |  3 +
 lib/cmdline/cmdline.c                  |  3 +-
 lib/cmdline/cmdline_cirbuf.c           |  1 -
 lib/cmdline/cmdline_private.h          | 49 ++++++++++++++
 lib/cmdline/cmdline_rdline.c           | 50 ++++++++++++++-
 lib/cmdline/cmdline_rdline.h           | 88 ++++++++++----------------
 lib/cmdline/version.map                |  8 ++-
 9 files changed, 154 insertions(+), 69 deletions(-)

diff --git a/app/test-cmdline/commands.c b/app/test-cmdline/commands.c
index d732976f08..a13e1d1afd 100644
--- a/app/test-cmdline/commands.c
+++ b/app/test-cmdline/commands.c
@@ -297,7 +297,7 @@ cmd_get_history_bufsize_parsed(__rte_unused void *parsed_result,
 	struct rdline *rdl = cmdline_get_rdline(cl);
 
 	cmdline_printf(cl, "History buffer size: %zu\n",
-			sizeof(rdl->history_buf));
+			rdline_get_history_buffer_size(rdl));
 }
 
 cmdline_parse_token_string_t cmd_get_history_bufsize_tok =
diff --git a/app/test/test_cmdline_lib.c b/app/test/test_cmdline_lib.c
index d5a09b4541..367dcad5be 100644
--- a/app/test/test_cmdline_lib.c
+++ b/app/test/test_cmdline_lib.c
@@ -83,18 +83,18 @@ test_cmdline_parse_fns(void)
 static int
 test_cmdline_rdline_fns(void)
 {
-	struct rdline rdl;
+	struct rdline *rdl = NULL;
 	rdline_write_char_t *wc = &cmdline_write_char;
 	rdline_validate_t *v = &valid_buffer;
 	rdline_complete_t *c = &complete_buffer;
 
-	if (rdline_init(NULL, wc, v, c) >= 0)
+	if (rdline_create(NULL, wc, v, c, NULL) >= 0)
 		goto error;
-	if (rdline_init(&rdl, NULL, v, c) >= 0)
+	if (rdline_create(&rdl, NULL, v, c, NULL) >= 0)
 		goto error;
-	if (rdline_init(&rdl, wc, NULL, c) >= 0)
+	if (rdline_create(&rdl, wc, NULL, c, NULL) >= 0)
 		goto error;
-	if (rdline_init(&rdl, wc, v, NULL) >= 0)
+	if (rdline_create(&rdl, wc, v, NULL, NULL) >= 0)
 		goto error;
 	if (rdline_char_in(NULL, 0) >= 0)
 		goto error;
@@ -102,25 +102,30 @@ test_cmdline_rdline_fns(void)
 		goto error;
 	if (rdline_add_history(NULL, "history") >= 0)
 		goto error;
-	if (rdline_add_history(&rdl, NULL) >= 0)
+	if (rdline_add_history(rdl, NULL) >= 0)
 		goto error;
 	if (rdline_get_history_item(NULL, 0) != NULL)
 		goto error;
 
 	/* void functions */
+	rdline_get_history_buffer_size(NULL);
+	rdline_get_opaque(NULL);
 	rdline_newline(NULL, "prompt");
-	rdline_newline(&rdl, NULL);
+	rdline_newline(rdl, NULL);
 	rdline_stop(NULL);
 	rdline_quit(NULL);
 	rdline_restart(NULL);
 	rdline_redisplay(NULL);
 	rdline_reset(NULL);
 	rdline_clear_history(NULL);
+	rdline_free(NULL);
 
+	rdline_free(rdl);
 	return 0;
 
 error:
 	printf("Error: function accepted null parameter!\n");
+	rdline_free(rdl);
 	return -1;
 }
 
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index 18377e5813..af11f4a656 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -103,6 +103,9 @@ API Changes
 
 * cmdline: Made ``cmdline`` structure definition hidden on Linux and FreeBSD.
 
+* cmdline: Made ``rdline`` structure definition hidden. Functions are added
+  to dynamically allocate and free it, and to access user data in callbacks.
+
 
 ABI Changes
 -----------
diff --git a/lib/cmdline/cmdline.c b/lib/cmdline/cmdline.c
index a176d15130..8f1854cb0b 100644
--- a/lib/cmdline/cmdline.c
+++ b/lib/cmdline/cmdline.c
@@ -85,13 +85,12 @@ cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out)
 	cl->ctx = ctx;
 
 	ret = rdline_init(&cl->rdl, cmdline_write_char, cmdline_valid_buffer,
-			cmdline_complete_buffer);
+			cmdline_complete_buffer, cl);
 	if (ret != 0) {
 		free(cl);
 		return NULL;
 	}
 
-	cl->rdl.opaque = cl;
 	cmdline_set_prompt(cl, prompt);
 	rdline_newline(&cl->rdl, cl->prompt);
 
diff --git a/lib/cmdline/cmdline_cirbuf.c b/lib/cmdline/cmdline_cirbuf.c
index 829a8af563..cbb76a7016 100644
--- a/lib/cmdline/cmdline_cirbuf.c
+++ b/lib/cmdline/cmdline_cirbuf.c
@@ -10,7 +10,6 @@
 
 #include "cmdline_cirbuf.h"
 
-
 int
 cirbuf_init(struct cirbuf *cbuf, char *buf, unsigned int start, unsigned int maxlen)
 {
diff --git a/lib/cmdline/cmdline_private.h b/lib/cmdline/cmdline_private.h
index 2e93674c66..c2e906d8de 100644
--- a/lib/cmdline/cmdline_private.h
+++ b/lib/cmdline/cmdline_private.h
@@ -17,6 +17,49 @@
 
 #include <cmdline.h>
 
+#define RDLINE_BUF_SIZE 512
+#define RDLINE_PROMPT_SIZE  32
+#define RDLINE_VT100_BUF_SIZE  8
+#define RDLINE_HISTORY_BUF_SIZE BUFSIZ
+#define RDLINE_HISTORY_MAX_LINE 64
+
+enum rdline_status {
+	RDLINE_INIT,
+	RDLINE_RUNNING,
+	RDLINE_EXITED
+};
+
+struct rdline {
+	enum rdline_status status;
+	/* rdline bufs */
+	struct cirbuf left;
+	struct cirbuf right;
+	char left_buf[RDLINE_BUF_SIZE+2]; /* reserve 2 chars for the \n\0 */
+	char right_buf[RDLINE_BUF_SIZE];
+
+	char prompt[RDLINE_PROMPT_SIZE];
+	unsigned int prompt_size;
+
+	char kill_buf[RDLINE_BUF_SIZE];
+	unsigned int kill_size;
+
+	/* history */
+	struct cirbuf history;
+	char history_buf[RDLINE_HISTORY_BUF_SIZE];
+	int history_cur_line;
+
+	/* callbacks and func pointers */
+	rdline_write_char_t *write_char;
+	rdline_validate_t *validate;
+	rdline_complete_t *complete;
+
+	/* vt100 parser */
+	struct cmdline_vt100 vt100;
+
+	/* opaque pointer */
+	void *opaque;
+};
+
 #ifdef RTE_EXEC_ENV_WINDOWS
 struct terminal {
 	DWORD input_mode;
@@ -57,4 +100,10 @@ ssize_t cmdline_read_char(struct cmdline *cl, char *c);
 __rte_format_printf(2, 0)
 int cmdline_vdprintf(int fd, const char *format, va_list op);
 
+int rdline_init(struct rdline *rdl,
+		rdline_write_char_t *write_char,
+		rdline_validate_t *validate,
+		rdline_complete_t *complete,
+		void *opaque);
+
 #endif
diff --git a/lib/cmdline/cmdline_rdline.c b/lib/cmdline/cmdline_rdline.c
index 2cb53e38f2..a525513465 100644
--- a/lib/cmdline/cmdline_rdline.c
+++ b/lib/cmdline/cmdline_rdline.c
@@ -13,6 +13,7 @@
 #include <ctype.h>
 
 #include "cmdline_cirbuf.h"
+#include "cmdline_private.h"
 #include "cmdline_rdline.h"
 
 static void rdline_puts(struct rdline *rdl, const char *buf);
@@ -37,9 +38,10 @@ isblank2(char c)
 
 int
 rdline_init(struct rdline *rdl,
-		 rdline_write_char_t *write_char,
-		 rdline_validate_t *validate,
-		 rdline_complete_t *complete)
+	    rdline_write_char_t *write_char,
+	    rdline_validate_t *validate,
+	    rdline_complete_t *complete,
+	    void *opaque)
 {
 	if (!rdl || !write_char || !validate || !complete)
 		return -EINVAL;
@@ -47,10 +49,40 @@ rdline_init(struct rdline *rdl,
 	rdl->validate = validate;
 	rdl->complete = complete;
 	rdl->write_char = write_char;
+	rdl->opaque = opaque;
 	rdl->status = RDLINE_INIT;
 	return cirbuf_init(&rdl->history, rdl->history_buf, 0, RDLINE_HISTORY_BUF_SIZE);
 }
 
+int
+rdline_create(struct rdline **out,
+	      rdline_write_char_t *write_char,
+	      rdline_validate_t *validate,
+	      rdline_complete_t *complete,
+	      void *opaque)
+{
+	struct rdline *rdl;
+	int ret;
+
+	if (out == NULL)
+		return -EINVAL;
+	rdl = malloc(sizeof(*rdl));
+	if (rdl == NULL)
+		return -ENOMEM;
+	ret = rdline_init(rdl, write_char, validate, complete, opaque);
+	if (ret < 0)
+		free(rdl);
+	else
+		*out = rdl;
+	return ret;
+}
+
+void
+rdline_free(struct rdline *rdl)
+{
+	free(rdl);
+}
+
 void
 rdline_newline(struct rdline *rdl, const char *prompt)
 {
@@ -564,6 +596,18 @@ rdline_get_history_item(struct rdline * rdl, unsigned int idx)
 	return NULL;
 }
 
+size_t
+rdline_get_history_buffer_size(struct rdline *rdl)
+{
+	return sizeof(rdl->history_buf);
+}
+
+void *
+rdline_get_opaque(struct rdline *rdl)
+{
+	return rdl != NULL ? rdl->opaque : NULL;
+}
+
 int
 rdline_add_history(struct rdline * rdl, const char * buf)
 {
diff --git a/lib/cmdline/cmdline_rdline.h b/lib/cmdline/cmdline_rdline.h
index d2170293de..b93e9ea569 100644
--- a/lib/cmdline/cmdline_rdline.h
+++ b/lib/cmdline/cmdline_rdline.h
@@ -10,9 +10,7 @@
 /**
  * This file is a small equivalent to the GNU readline library, but it
  * was originally designed for small systems, like Atmel AVR
- * microcontrollers (8 bits). Indeed, we don't use any malloc that is
- * sometimes not implemented (or just not recommended) on such
- * systems.
+ * microcontrollers (8 bits). It only uses malloc() on object creation.
  *
  * Obviously, it does not support as many things as the GNU readline,
  * but at least it supports some interesting features like a kill
@@ -31,6 +29,7 @@
  */
 
 #include <stdio.h>
+#include <rte_compat.h>
 #include <cmdline_cirbuf.h>
 #include <cmdline_vt100.h>
 
@@ -38,19 +37,6 @@
 extern "C" {
 #endif
 
-/* configuration */
-#define RDLINE_BUF_SIZE 512
-#define RDLINE_PROMPT_SIZE  32
-#define RDLINE_VT100_BUF_SIZE  8
-#define RDLINE_HISTORY_BUF_SIZE BUFSIZ
-#define RDLINE_HISTORY_MAX_LINE 64
-
-enum rdline_status {
-	RDLINE_INIT,
-	RDLINE_RUNNING,
-	RDLINE_EXITED
-};
-
 struct rdline;
 
 typedef int (rdline_write_char_t)(struct rdline *rdl, char);
@@ -60,52 +46,34 @@ typedef int (rdline_complete_t)(struct rdline *rdl, const char *buf,
 				char *dstbuf, unsigned int dstsize,
 				int *state);
 
-struct rdline {
-	enum rdline_status status;
-	/* rdline bufs */
-	struct cirbuf left;
-	struct cirbuf right;
-	char left_buf[RDLINE_BUF_SIZE+2]; /* reserve 2 chars for the \n\0 */
-	char right_buf[RDLINE_BUF_SIZE];
-
-	char prompt[RDLINE_PROMPT_SIZE];
-	unsigned int prompt_size;
-
-	char kill_buf[RDLINE_BUF_SIZE];
-	unsigned int kill_size;
-
-	/* history */
-	struct cirbuf history;
-	char history_buf[RDLINE_HISTORY_BUF_SIZE];
-	int history_cur_line;
-
-	/* callbacks and func pointers */
-	rdline_write_char_t *write_char;
-	rdline_validate_t *validate;
-	rdline_complete_t *complete;
-
-	/* vt100 parser */
-	struct cmdline_vt100 vt100;
-
-	/* opaque pointer */
-	void *opaque;
-};
-
 /**
- * Init fields for a struct rdline. Call this only once at the beginning
- * of your program.
- * \param rdl A pointer to an uninitialized struct rdline
+ * Allocate and initialize a new rdline instance.
+ *
+ * \param rdl Receives a pointer to the allocated structure.
  * \param write_char The function used by the function to write a character
  * \param validate A pointer to the function to execute when the
  *                 user validates the buffer.
  * \param complete A pointer to the function to execute when the
  *                 user completes the buffer.
+ * \param opaque User data for use in the callbacks.
+ *
+ * \return 0 on success, negative errno-style code in failure.
  */
-int rdline_init(struct rdline *rdl,
-		 rdline_write_char_t *write_char,
-		 rdline_validate_t *validate,
-		 rdline_complete_t *complete);
+__rte_experimental
+int rdline_create(struct rdline **rdl,
+		  rdline_write_char_t *write_char,
+		  rdline_validate_t *validate,
+		  rdline_complete_t *complete,
+		  void *opaque);
 
+/**
+ * Free an rdline instance.
+ *
+ * \param rdl A pointer to an initialized struct rdline.
+ *            If NULL, this function is a no-op.
+ */
+__rte_experimental
+void rdline_free(struct rdline *rdl);
 
 /**
  * Init the current buffer, and display a prompt.
@@ -194,6 +162,18 @@ void rdline_clear_history(struct rdline *rdl);
  */
 char *rdline_get_history_item(struct rdline *rdl, unsigned int i);
 
+/**
+ * Get maximum history buffer size.
+ */
+__rte_experimental
+size_t rdline_get_history_buffer_size(struct rdline *rdl);
+
+/**
+ * Get the opaque pointer supplied on struct rdline creation.
+ */
+__rte_experimental
+void *rdline_get_opaque(struct rdline *rdl);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/cmdline/version.map b/lib/cmdline/version.map
index 980adb4f23..13feb4d0a5 100644
--- a/lib/cmdline/version.map
+++ b/lib/cmdline/version.map
@@ -57,7 +57,6 @@ DPDK_22 {
 	rdline_clear_history;
 	rdline_get_buffer;
 	rdline_get_history_item;
-	rdline_init;
 	rdline_newline;
 	rdline_quit;
 	rdline_redisplay;
@@ -73,7 +72,14 @@ DPDK_22 {
 EXPERIMENTAL {
 	global:
 
+	# added in 20.11
 	cmdline_get_rdline;
 
+	# added in 21.11
+	rdline_create;
+	rdline_free;
+	rdline_get_history_buffer_size;
+	rdline_get_opaque;
+
 	local: *;
 };
-- 
2.29.3

Re: [dpdk-dev] [PATCH v3 2/2] cmdline: make struct rdline opaque

[Olivier Matz]

Hi Dmitry,

Few comments below.

On Tue, Oct 05, 2021 at 03:55:16AM +0300, Dmitry Kozlyuk wrote:
> Hide struct rdline definition and some RDLINE_* constants in order
> to be able to change internal buffer sizes transparently to the user.
> Add new functions:
> 
> * rdline_create(): allocate and initialize struct rdline.
>   This function replaces rdline_init() and takes an extra parameter:
>   opaque user data for the callbacks.
> * rdline_free(): deallocate struct rdline.
> * rdline_get_history_buffer_size(): for use in tests.
> * rdline_get_opaque(): to obtain user data in callback functions.
> 
> Remove rdline_init() function from library headers and export list,
> because using it requires the knowledge of sizeof(struct rdline).
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  app/test-cmdline/commands.c            |  2 +-
>  app/test/test_cmdline_lib.c            | 19 ++++--
>  doc/guides/rel_notes/release_21_11.rst |  3 +
>  lib/cmdline/cmdline.c                  |  3 +-
>  lib/cmdline/cmdline_cirbuf.c           |  1 -
>  lib/cmdline/cmdline_private.h          | 49 ++++++++++++++
>  lib/cmdline/cmdline_rdline.c           | 50 ++++++++++++++-
>  lib/cmdline/cmdline_rdline.h           | 88 ++++++++++----------------
>  lib/cmdline/version.map                |  8 ++-
>  9 files changed, 154 insertions(+), 69 deletions(-)
> 
> diff --git a/app/test-cmdline/commands.c b/app/test-cmdline/commands.c
> index d732976f08..a13e1d1afd 100644
> --- a/app/test-cmdline/commands.c
> +++ b/app/test-cmdline/commands.c
> @@ -297,7 +297,7 @@ cmd_get_history_bufsize_parsed(__rte_unused void *parsed_result,
>  	struct rdline *rdl = cmdline_get_rdline(cl);
>  
>  	cmdline_printf(cl, "History buffer size: %zu\n",
> -			sizeof(rdl->history_buf));
> +			rdline_get_history_buffer_size(rdl));

My first impression was that having this function just for the tests was
not really useful.

But thinking a bit more about it, now that the structure is hidden, we
could have an API in the future to change the size of the history
buffer, in which case this function makes sense.

[...]

> diff --git a/lib/cmdline/cmdline_cirbuf.c b/lib/cmdline/cmdline_cirbuf.c
> index 829a8af563..cbb76a7016 100644
> --- a/lib/cmdline/cmdline_cirbuf.c
> +++ b/lib/cmdline/cmdline_cirbuf.c
> @@ -10,7 +10,6 @@
>  
>  #include "cmdline_cirbuf.h"
>  
> -
>  int
>  cirbuf_init(struct cirbuf *cbuf, char *buf, unsigned int start, unsigned int maxlen)
>  {

unexpected change

[...]

> --- a/lib/cmdline/cmdline_rdline.c
> +++ b/lib/cmdline/cmdline_rdline.c
> @@ -13,6 +13,7 @@
>  #include <ctype.h>
>  
>  #include "cmdline_cirbuf.h"
> +#include "cmdline_private.h"
>  #include "cmdline_rdline.h"
>  
>  static void rdline_puts(struct rdline *rdl, const char *buf);
> @@ -37,9 +38,10 @@ isblank2(char c)
>  
>  int
>  rdline_init(struct rdline *rdl,
> -		 rdline_write_char_t *write_char,
> -		 rdline_validate_t *validate,
> -		 rdline_complete_t *complete)
> +	    rdline_write_char_t *write_char,
> +	    rdline_validate_t *validate,
> +	    rdline_complete_t *complete,
> +	    void *opaque)
>  {
>  	if (!rdl || !write_char || !validate || !complete)
>  		return -EINVAL;
> @@ -47,10 +49,40 @@ rdline_init(struct rdline *rdl,
>  	rdl->validate = validate;
>  	rdl->complete = complete;
>  	rdl->write_char = write_char;
> +	rdl->opaque = opaque;
>  	rdl->status = RDLINE_INIT;
>  	return cirbuf_init(&rdl->history, rdl->history_buf, 0, RDLINE_HISTORY_BUF_SIZE);
>  }
>  
> +int
> +rdline_create(struct rdline **out,
> +	      rdline_write_char_t *write_char,
> +	      rdline_validate_t *validate,
> +	      rdline_complete_t *complete,
> +	      void *opaque)
> +{

For consistency, wouldn't it be better to keep the same model than
cmdline_new()? I mean return a pointer and name it rdline_new().


> +	struct rdline *rdl;
> +	int ret;
> +
> +	if (out == NULL)
> +		return -EINVAL;
> +	rdl = malloc(sizeof(*rdl));
> +	if (rdl == NULL)
> +		return -ENOMEM;
> +	ret = rdline_init(rdl, write_char, validate, complete, opaque);
> +	if (ret < 0)
> +		free(rdl);
> +	else
> +		*out = rdl;
> +	return ret;
> +}

[...]

> +size_t
> +rdline_get_history_buffer_size(struct rdline *rdl)
> +{
> +	return sizeof(rdl->history_buf);
> +}
> +
> +void *
> +rdline_get_opaque(struct rdline *rdl)
> +{
> +	return rdl != NULL ? rdl->opaque : NULL;
> +}

rdline_get_opaque() is safe when rdl is NULL, but
rdline_get_history_buffer_size() is not.

To me, both are acceptable but I'd prefer to have the same behavior
for these 2 functions.


Apart from that, looks good to me.

Thanks!
Olivier

Re: [dpdk-dev] [PATCH v3 2/2] cmdline: make struct rdline opaque

[Dmitry Kozlyuk]

Hi Olivier,

Thanks for the review, please see below.

2021-10-05 10:27 (UTC+0200), Olivier Matz:
> [...]
> > diff --git a/lib/cmdline/cmdline_cirbuf.c b/lib/cmdline/cmdline_cirbuf.c
> > index 829a8af563..cbb76a7016 100644
> > --- a/lib/cmdline/cmdline_cirbuf.c
> > +++ b/lib/cmdline/cmdline_cirbuf.c
> > @@ -10,7 +10,6 @@
> >  
> >  #include "cmdline_cirbuf.h"
> >  
> > -
> >  int
> >  cirbuf_init(struct cirbuf *cbuf, char *buf, unsigned int start, unsigned int maxlen)
> >  {  
> 
> unexpected change

Will remove in v4.

> [...]
> 
> > --- a/lib/cmdline/cmdline_rdline.c
> > +++ b/lib/cmdline/cmdline_rdline.c
> > @@ -13,6 +13,7 @@
> >  #include <ctype.h>
> >  
> >  #include "cmdline_cirbuf.h"
> > +#include "cmdline_private.h"
> >  #include "cmdline_rdline.h"
> >  
> >  static void rdline_puts(struct rdline *rdl, const char *buf);
> > @@ -37,9 +38,10 @@ isblank2(char c)
> >  
> >  int
> >  rdline_init(struct rdline *rdl,
> > -		 rdline_write_char_t *write_char,
> > -		 rdline_validate_t *validate,
> > -		 rdline_complete_t *complete)
> > +	    rdline_write_char_t *write_char,
> > +	    rdline_validate_t *validate,
> > +	    rdline_complete_t *complete,
> > +	    void *opaque)
> >  {
> >  	if (!rdl || !write_char || !validate || !complete)
> >  		return -EINVAL;
> > @@ -47,10 +49,40 @@ rdline_init(struct rdline *rdl,
> >  	rdl->validate = validate;
> >  	rdl->complete = complete;
> >  	rdl->write_char = write_char;
> > +	rdl->opaque = opaque;
> >  	rdl->status = RDLINE_INIT;
> >  	return cirbuf_init(&rdl->history, rdl->history_buf, 0, RDLINE_HISTORY_BUF_SIZE);
> >  }
> >  
> > +int
> > +rdline_create(struct rdline **out,
> > +	      rdline_write_char_t *write_char,
> > +	      rdline_validate_t *validate,
> > +	      rdline_complete_t *complete,
> > +	      void *opaque)
> > +{  
> 
> For consistency, wouldn't it be better to keep the same model than
> cmdline_new()? I mean return a pointer and name it rdline_new().

If we don't really need to distinguish EINVAL and ENOMEM errors here,
then I agree. Otherwise, do you propose to return the error code via
rte_errno? Currenly no cmdline functions use it. This would also add a
runtime dependency on EAL (currently cmdline only depends on its headers).

> [...]
> > +size_t
> > +rdline_get_history_buffer_size(struct rdline *rdl)
> > +{
> > +	return sizeof(rdl->history_buf);
> > +}
> > +
> > +void *
> > +rdline_get_opaque(struct rdline *rdl)
> > +{
> > +	return rdl != NULL ? rdl->opaque : NULL;
> > +}  
> 
> rdline_get_opaque() is safe when rdl is NULL, but
> rdline_get_history_buffer_size() is not.
> 
> To me, both are acceptable but I'd prefer to have the same behavior
> for these 2 functions.

rdline_get_history_buffer_size() is safe because sizeof() is evaluated at
compile time. There's a unit test checking that all functions are NULL-safe.

Re: [dpdk-dev] [PATCH v3 2/2] cmdline: make struct rdline opaque

[Olivier Matz]

On Tue, Oct 05, 2021 at 12:03:01PM +0300, Dmitry Kozlyuk wrote:
> Hi Olivier,
> 
> Thanks for the review, please see below.
> 
> 2021-10-05 10:27 (UTC+0200), Olivier Matz:
> > [...]
> > > diff --git a/lib/cmdline/cmdline_cirbuf.c b/lib/cmdline/cmdline_cirbuf.c
> > > index 829a8af563..cbb76a7016 100644
> > > --- a/lib/cmdline/cmdline_cirbuf.c
> > > +++ b/lib/cmdline/cmdline_cirbuf.c
> > > @@ -10,7 +10,6 @@
> > >  
> > >  #include "cmdline_cirbuf.h"
> > >  
> > > -
> > >  int
> > >  cirbuf_init(struct cirbuf *cbuf, char *buf, unsigned int start, unsigned int maxlen)
> > >  {  
> > 
> > unexpected change
> 
> Will remove in v4.
> 
> > [...]
> > 
> > > --- a/lib/cmdline/cmdline_rdline.c
> > > +++ b/lib/cmdline/cmdline_rdline.c
> > > @@ -13,6 +13,7 @@
> > >  #include <ctype.h>
> > >  
> > >  #include "cmdline_cirbuf.h"
> > > +#include "cmdline_private.h"
> > >  #include "cmdline_rdline.h"
> > >  
> > >  static void rdline_puts(struct rdline *rdl, const char *buf);
> > > @@ -37,9 +38,10 @@ isblank2(char c)
> > >  
> > >  int
> > >  rdline_init(struct rdline *rdl,
> > > -		 rdline_write_char_t *write_char,
> > > -		 rdline_validate_t *validate,
> > > -		 rdline_complete_t *complete)
> > > +	    rdline_write_char_t *write_char,
> > > +	    rdline_validate_t *validate,
> > > +	    rdline_complete_t *complete,
> > > +	    void *opaque)
> > >  {
> > >  	if (!rdl || !write_char || !validate || !complete)
> > >  		return -EINVAL;
> > > @@ -47,10 +49,40 @@ rdline_init(struct rdline *rdl,
> > >  	rdl->validate = validate;
> > >  	rdl->complete = complete;
> > >  	rdl->write_char = write_char;
> > > +	rdl->opaque = opaque;
> > >  	rdl->status = RDLINE_INIT;
> > >  	return cirbuf_init(&rdl->history, rdl->history_buf, 0, RDLINE_HISTORY_BUF_SIZE);
> > >  }
> > >  
> > > +int
> > > +rdline_create(struct rdline **out,
> > > +	      rdline_write_char_t *write_char,
> > > +	      rdline_validate_t *validate,
> > > +	      rdline_complete_t *complete,
> > > +	      void *opaque)
> > > +{  
> > 
> > For consistency, wouldn't it be better to keep the same model than
> > cmdline_new()? I mean return a pointer and name it rdline_new().
> 
> If we don't really need to distinguish EINVAL and ENOMEM errors here,
> then I agree. Otherwise, do you propose to return the error code via
> rte_errno? Currenly no cmdline functions use it. This would also add a
> runtime dependency on EAL (currently cmdline only depends on its headers).

Good point, I was indeed thinking about NULL + rte_errno, but I did not
anticipate the new dependency to eal.

Given there's no errno in cmdline_new(), which is the main user API, I think
we can do the same for rdline_new().


> > [...]
> > > +size_t
> > > +rdline_get_history_buffer_size(struct rdline *rdl)
> > > +{
> > > +	return sizeof(rdl->history_buf);
> > > +}
> > > +
> > > +void *
> > > +rdline_get_opaque(struct rdline *rdl)
> > > +{
> > > +	return rdl != NULL ? rdl->opaque : NULL;
> > > +}  
> > 
> > rdline_get_opaque() is safe when rdl is NULL, but
> > rdline_get_history_buffer_size() is not.
> > 
> > To me, both are acceptable but I'd prefer to have the same behavior
> > for these 2 functions.
> 
> rdline_get_history_buffer_size() is safe because sizeof() is evaluated at
> compile time. There's a unit test checking that all functions are NULL-safe.

Oh yes, of course, thanks.

[dpdk-dev] [PATCH v4 0/2] cmdline: reduce ABI

[Dmitry Kozlyuk]

Hide struct cmdline following the deprecation notice.
Hide struct rdline following the v1 discussion.

v4: rdline_create -> rdline_new, restore empty line (Olivier).
v3: add experimental tags and releae notes for rdline.
v2: also hide struct rdline (David, Olivier).
 
Dmitry Kozlyuk (2):
  cmdline: make struct cmdline opaque
  cmdline: make struct rdline opaque

 app/test-cmdline/commands.c            |  2 +-
 app/test/test_cmdline_lib.c            | 22 ++++---
 doc/guides/rel_notes/deprecation.rst   |  4 --
 doc/guides/rel_notes/release_21_11.rst |  5 ++
 lib/cmdline/cmdline.c                  |  3 +-
 lib/cmdline/cmdline.h                  | 19 ------
 lib/cmdline/cmdline_private.h          | 57 ++++++++++++++++-
 lib/cmdline/cmdline_rdline.c           | 43 ++++++++++++-
 lib/cmdline/cmdline_rdline.h           | 87 ++++++++++----------------
 lib/cmdline/version.map                |  8 ++-
 10 files changed, 157 insertions(+), 93 deletions(-)

-- 
2.29.3

[dpdk-dev] [PATCH v4 1/2] cmdline: make struct cmdline opaque

[Dmitry Kozlyuk]

Remove the definition of `struct cmdline` from public header.
Deprecation notice:
https://mails.dpdk.org/archives/dev/2020-September/183310.html

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: David Marchand <david.marchand@redhat.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
---
 doc/guides/rel_notes/deprecation.rst   |  4 ----
 doc/guides/rel_notes/release_21_11.rst |  2 ++
 lib/cmdline/cmdline.h                  | 19 -------------------
 lib/cmdline/cmdline_private.h          |  8 +++++++-
 4 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 76a4abfd6b..a404276fa2 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -275,10 +275,6 @@ Deprecation Notices
 * metrics: The function ``rte_metrics_init`` will have a non-void return
   in order to notify errors instead of calling ``rte_exit``.
 
-* cmdline: ``cmdline`` structure will be made opaque to hide platform-specific
-  content. On Linux and FreeBSD, supported prior to DPDK 20.11,
-  original structure will be kept until DPDK 21.11.
-
 * security: The functions ``rte_security_set_pkt_metadata`` and
   ``rte_security_get_userdata`` will be made inline functions and additional
   flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index b55900936d..18377e5813 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -101,6 +101,8 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* cmdline: Made ``cmdline`` structure definition hidden on Linux and FreeBSD.
+
 
 ABI Changes
 -----------
diff --git a/lib/cmdline/cmdline.h b/lib/cmdline/cmdline.h
index c29762ddae..96674dfda2 100644
--- a/lib/cmdline/cmdline.h
+++ b/lib/cmdline/cmdline.h
@@ -7,10 +7,6 @@
 #ifndef _CMDLINE_H_
 #define _CMDLINE_H_
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-#include <termios.h>
-#endif
-
 #include <rte_common.h>
 #include <rte_compat.h>
 
@@ -27,23 +23,8 @@
 extern "C" {
 #endif
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-
-struct cmdline {
-	int s_in;
-	int s_out;
-	cmdline_parse_ctx_t *ctx;
-	struct rdline rdl;
-	char prompt[RDLINE_PROMPT_SIZE];
-	struct termios oldterm;
-};
-
-#else
-
 struct cmdline;
 
-#endif /* RTE_EXEC_ENV_WINDOWS */
-
 struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
 void cmdline_set_prompt(struct cmdline *cl, const char *prompt);
 void cmdline_free(struct cmdline *cl);
diff --git a/lib/cmdline/cmdline_private.h b/lib/cmdline/cmdline_private.h
index a87c45275c..2e93674c66 100644
--- a/lib/cmdline/cmdline_private.h
+++ b/lib/cmdline/cmdline_private.h
@@ -11,6 +11,8 @@
 #include <rte_os_shim.h>
 #ifdef RTE_EXEC_ENV_WINDOWS
 #include <rte_windows.h>
+#else
+#include <termios.h>
 #endif
 
 #include <cmdline.h>
@@ -22,6 +24,7 @@ struct terminal {
 	int is_console_input;
 	int is_console_output;
 };
+#endif
 
 struct cmdline {
 	int s_in;
@@ -29,11 +32,14 @@ struct cmdline {
 	cmdline_parse_ctx_t *ctx;
 	struct rdline rdl;
 	char prompt[RDLINE_PROMPT_SIZE];
+#ifdef RTE_EXEC_ENV_WINDOWS
 	struct terminal oldterm;
 	char repeated_char;
 	WORD repeat_count;
-};
+#else
+	struct termios oldterm;
 #endif
+};
 
 /* Disable buffering and echoing, save previous settings to oldterm. */
 void terminal_adjust(struct cmdline *cl);
-- 
2.29.3

[dpdk-dev] [PATCH v4 2/2] cmdline: make struct rdline opaque

[Dmitry Kozlyuk]

Hide struct rdline definition and some RDLINE_* constants in order
to be able to change internal buffer sizes transparently to the user.
Add new functions:

* rdline_new(): allocate and initialize struct rdline.
  This function replaces rdline_init() and takes an extra parameter:
  opaque user data for the callbacks.
* rdline_free(): deallocate struct rdline.
* rdline_get_history_buffer_size(): for use in tests.
* rdline_get_opaque(): to obtain user data in callback functions.

Remove rdline_init() function from library headers and export list,
because using it requires the knowledge of sizeof(struct rdline).

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 app/test-cmdline/commands.c            |  2 +-
 app/test/test_cmdline_lib.c            | 22 ++++---
 doc/guides/rel_notes/release_21_11.rst |  3 +
 lib/cmdline/cmdline.c                  |  3 +-
 lib/cmdline/cmdline_private.h          | 49 +++++++++++++++
 lib/cmdline/cmdline_rdline.c           | 43 ++++++++++++-
 lib/cmdline/cmdline_rdline.h           | 87 ++++++++++----------------
 lib/cmdline/version.map                |  8 ++-
 8 files changed, 148 insertions(+), 69 deletions(-)

diff --git a/app/test-cmdline/commands.c b/app/test-cmdline/commands.c
index d732976f08..a13e1d1afd 100644
--- a/app/test-cmdline/commands.c
+++ b/app/test-cmdline/commands.c
@@ -297,7 +297,7 @@ cmd_get_history_bufsize_parsed(__rte_unused void *parsed_result,
 	struct rdline *rdl = cmdline_get_rdline(cl);
 
 	cmdline_printf(cl, "History buffer size: %zu\n",
-			sizeof(rdl->history_buf));
+			rdline_get_history_buffer_size(rdl));
 }
 
 cmdline_parse_token_string_t cmd_get_history_bufsize_tok =
diff --git a/app/test/test_cmdline_lib.c b/app/test/test_cmdline_lib.c
index d5a09b4541..24bc03fccb 100644
--- a/app/test/test_cmdline_lib.c
+++ b/app/test/test_cmdline_lib.c
@@ -83,18 +83,19 @@ test_cmdline_parse_fns(void)
 static int
 test_cmdline_rdline_fns(void)
 {
-	struct rdline rdl;
+	struct rdline *rdl = NULL;
 	rdline_write_char_t *wc = &cmdline_write_char;
 	rdline_validate_t *v = &valid_buffer;
 	rdline_complete_t *c = &complete_buffer;
 
-	if (rdline_init(NULL, wc, v, c) >= 0)
+	rdl = rdline_new(NULL, v, c, NULL);
+	if (rdl != NULL)
 		goto error;
-	if (rdline_init(&rdl, NULL, v, c) >= 0)
+	rdl = rdline_new(wc, NULL, c, NULL);
+	if (rdl != NULL)
 		goto error;
-	if (rdline_init(&rdl, wc, NULL, c) >= 0)
-		goto error;
-	if (rdline_init(&rdl, wc, v, NULL) >= 0)
+	rdl = rdline_new(wc, v, NULL, NULL);
+	if (rdl != NULL)
 		goto error;
 	if (rdline_char_in(NULL, 0) >= 0)
 		goto error;
@@ -102,25 +103,30 @@ test_cmdline_rdline_fns(void)
 		goto error;
 	if (rdline_add_history(NULL, "history") >= 0)
 		goto error;
-	if (rdline_add_history(&rdl, NULL) >= 0)
+	if (rdline_add_history(rdl, NULL) >= 0)
 		goto error;
 	if (rdline_get_history_item(NULL, 0) != NULL)
 		goto error;
 
 	/* void functions */
+	rdline_get_history_buffer_size(NULL);
+	rdline_get_opaque(NULL);
 	rdline_newline(NULL, "prompt");
-	rdline_newline(&rdl, NULL);
+	rdline_newline(rdl, NULL);
 	rdline_stop(NULL);
 	rdline_quit(NULL);
 	rdline_restart(NULL);
 	rdline_redisplay(NULL);
 	rdline_reset(NULL);
 	rdline_clear_history(NULL);
+	rdline_free(NULL);
 
+	rdline_free(rdl);
 	return 0;
 
 error:
 	printf("Error: function accepted null parameter!\n");
+	rdline_free(rdl);
 	return -1;
 }
 
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index 18377e5813..af11f4a656 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -103,6 +103,9 @@ API Changes
 
 * cmdline: Made ``cmdline`` structure definition hidden on Linux and FreeBSD.
 
+* cmdline: Made ``rdline`` structure definition hidden. Functions are added
+  to dynamically allocate and free it, and to access user data in callbacks.
+
 
 ABI Changes
 -----------
diff --git a/lib/cmdline/cmdline.c b/lib/cmdline/cmdline.c
index a176d15130..8f1854cb0b 100644
--- a/lib/cmdline/cmdline.c
+++ b/lib/cmdline/cmdline.c
@@ -85,13 +85,12 @@ cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out)
 	cl->ctx = ctx;
 
 	ret = rdline_init(&cl->rdl, cmdline_write_char, cmdline_valid_buffer,
-			cmdline_complete_buffer);
+			cmdline_complete_buffer, cl);
 	if (ret != 0) {
 		free(cl);
 		return NULL;
 	}
 
-	cl->rdl.opaque = cl;
 	cmdline_set_prompt(cl, prompt);
 	rdline_newline(&cl->rdl, cl->prompt);
 
diff --git a/lib/cmdline/cmdline_private.h b/lib/cmdline/cmdline_private.h
index 2e93674c66..c2e906d8de 100644
--- a/lib/cmdline/cmdline_private.h
+++ b/lib/cmdline/cmdline_private.h
@@ -17,6 +17,49 @@
 
 #include <cmdline.h>
 
+#define RDLINE_BUF_SIZE 512
+#define RDLINE_PROMPT_SIZE  32
+#define RDLINE_VT100_BUF_SIZE  8
+#define RDLINE_HISTORY_BUF_SIZE BUFSIZ
+#define RDLINE_HISTORY_MAX_LINE 64
+
+enum rdline_status {
+	RDLINE_INIT,
+	RDLINE_RUNNING,
+	RDLINE_EXITED
+};
+
+struct rdline {
+	enum rdline_status status;
+	/* rdline bufs */
+	struct cirbuf left;
+	struct cirbuf right;
+	char left_buf[RDLINE_BUF_SIZE+2]; /* reserve 2 chars for the \n\0 */
+	char right_buf[RDLINE_BUF_SIZE];
+
+	char prompt[RDLINE_PROMPT_SIZE];
+	unsigned int prompt_size;
+
+	char kill_buf[RDLINE_BUF_SIZE];
+	unsigned int kill_size;
+
+	/* history */
+	struct cirbuf history;
+	char history_buf[RDLINE_HISTORY_BUF_SIZE];
+	int history_cur_line;
+
+	/* callbacks and func pointers */
+	rdline_write_char_t *write_char;
+	rdline_validate_t *validate;
+	rdline_complete_t *complete;
+
+	/* vt100 parser */
+	struct cmdline_vt100 vt100;
+
+	/* opaque pointer */
+	void *opaque;
+};
+
 #ifdef RTE_EXEC_ENV_WINDOWS
 struct terminal {
 	DWORD input_mode;
@@ -57,4 +100,10 @@ ssize_t cmdline_read_char(struct cmdline *cl, char *c);
 __rte_format_printf(2, 0)
 int cmdline_vdprintf(int fd, const char *format, va_list op);
 
+int rdline_init(struct rdline *rdl,
+		rdline_write_char_t *write_char,
+		rdline_validate_t *validate,
+		rdline_complete_t *complete,
+		void *opaque);
+
 #endif
diff --git a/lib/cmdline/cmdline_rdline.c b/lib/cmdline/cmdline_rdline.c
index 2cb53e38f2..d92b1cda53 100644
--- a/lib/cmdline/cmdline_rdline.c
+++ b/lib/cmdline/cmdline_rdline.c
@@ -13,6 +13,7 @@
 #include <ctype.h>
 
 #include "cmdline_cirbuf.h"
+#include "cmdline_private.h"
 #include "cmdline_rdline.h"
 
 static void rdline_puts(struct rdline *rdl, const char *buf);
@@ -37,9 +38,10 @@ isblank2(char c)
 
 int
 rdline_init(struct rdline *rdl,
-		 rdline_write_char_t *write_char,
-		 rdline_validate_t *validate,
-		 rdline_complete_t *complete)
+	    rdline_write_char_t *write_char,
+	    rdline_validate_t *validate,
+	    rdline_complete_t *complete,
+	    void *opaque)
 {
 	if (!rdl || !write_char || !validate || !complete)
 		return -EINVAL;
@@ -47,10 +49,33 @@ rdline_init(struct rdline *rdl,
 	rdl->validate = validate;
 	rdl->complete = complete;
 	rdl->write_char = write_char;
+	rdl->opaque = opaque;
 	rdl->status = RDLINE_INIT;
 	return cirbuf_init(&rdl->history, rdl->history_buf, 0, RDLINE_HISTORY_BUF_SIZE);
 }
 
+struct rdline *
+rdline_new(rdline_write_char_t *write_char,
+	   rdline_validate_t *validate,
+	   rdline_complete_t *complete,
+	   void *opaque)
+{
+	struct rdline *rdl;
+
+	rdl = malloc(sizeof(*rdl));
+	if (rdline_init(rdl, write_char, validate, complete, opaque) < 0) {
+		free(rdl);
+		rdl = NULL;
+	}
+	return rdl;
+}
+
+void
+rdline_free(struct rdline *rdl)
+{
+	free(rdl);
+}
+
 void
 rdline_newline(struct rdline *rdl, const char *prompt)
 {
@@ -564,6 +589,18 @@ rdline_get_history_item(struct rdline * rdl, unsigned int idx)
 	return NULL;
 }
 
+size_t
+rdline_get_history_buffer_size(struct rdline *rdl)
+{
+	return sizeof(rdl->history_buf);
+}
+
+void *
+rdline_get_opaque(struct rdline *rdl)
+{
+	return rdl != NULL ? rdl->opaque : NULL;
+}
+
 int
 rdline_add_history(struct rdline * rdl, const char * buf)
 {
diff --git a/lib/cmdline/cmdline_rdline.h b/lib/cmdline/cmdline_rdline.h
index d2170293de..af66b70495 100644
--- a/lib/cmdline/cmdline_rdline.h
+++ b/lib/cmdline/cmdline_rdline.h
@@ -10,9 +10,7 @@
 /**
  * This file is a small equivalent to the GNU readline library, but it
  * was originally designed for small systems, like Atmel AVR
- * microcontrollers (8 bits). Indeed, we don't use any malloc that is
- * sometimes not implemented (or just not recommended) on such
- * systems.
+ * microcontrollers (8 bits). It only uses malloc() on object creation.
  *
  * Obviously, it does not support as many things as the GNU readline,
  * but at least it supports some interesting features like a kill
@@ -31,6 +29,7 @@
  */
 
 #include <stdio.h>
+#include <rte_compat.h>
 #include <cmdline_cirbuf.h>
 #include <cmdline_vt100.h>
 
@@ -38,19 +37,6 @@
 extern "C" {
 #endif
 
-/* configuration */
-#define RDLINE_BUF_SIZE 512
-#define RDLINE_PROMPT_SIZE  32
-#define RDLINE_VT100_BUF_SIZE  8
-#define RDLINE_HISTORY_BUF_SIZE BUFSIZ
-#define RDLINE_HISTORY_MAX_LINE 64
-
-enum rdline_status {
-	RDLINE_INIT,
-	RDLINE_RUNNING,
-	RDLINE_EXITED
-};
-
 struct rdline;
 
 typedef int (rdline_write_char_t)(struct rdline *rdl, char);
@@ -60,52 +46,33 @@ typedef int (rdline_complete_t)(struct rdline *rdl, const char *buf,
 				char *dstbuf, unsigned int dstsize,
 				int *state);
 
-struct rdline {
-	enum rdline_status status;
-	/* rdline bufs */
-	struct cirbuf left;
-	struct cirbuf right;
-	char left_buf[RDLINE_BUF_SIZE+2]; /* reserve 2 chars for the \n\0 */
-	char right_buf[RDLINE_BUF_SIZE];
-
-	char prompt[RDLINE_PROMPT_SIZE];
-	unsigned int prompt_size;
-
-	char kill_buf[RDLINE_BUF_SIZE];
-	unsigned int kill_size;
-
-	/* history */
-	struct cirbuf history;
-	char history_buf[RDLINE_HISTORY_BUF_SIZE];
-	int history_cur_line;
-
-	/* callbacks and func pointers */
-	rdline_write_char_t *write_char;
-	rdline_validate_t *validate;
-	rdline_complete_t *complete;
-
-	/* vt100 parser */
-	struct cmdline_vt100 vt100;
-
-	/* opaque pointer */
-	void *opaque;
-};
-
 /**
- * Init fields for a struct rdline. Call this only once at the beginning
- * of your program.
- * \param rdl A pointer to an uninitialized struct rdline
+ * Allocate and initialize a new rdline instance.
+ *
+ * \param rdl Receives a pointer to the allocated structure.
  * \param write_char The function used by the function to write a character
  * \param validate A pointer to the function to execute when the
  *                 user validates the buffer.
  * \param complete A pointer to the function to execute when the
  *                 user completes the buffer.
+ * \param opaque User data for use in the callbacks.
+ *
+ * \return 0 on success, negative errno-style code in failure.
  */
-int rdline_init(struct rdline *rdl,
-		 rdline_write_char_t *write_char,
-		 rdline_validate_t *validate,
-		 rdline_complete_t *complete);
+__rte_experimental
+struct rdline *rdline_new(rdline_write_char_t *write_char,
+			  rdline_validate_t *validate,
+			  rdline_complete_t *complete,
+			  void *opaque);
 
+/**
+ * Free an rdline instance.
+ *
+ * \param rdl A pointer to an initialized struct rdline.
+ *            If NULL, this function is a no-op.
+ */
+__rte_experimental
+void rdline_free(struct rdline *rdl);
 
 /**
  * Init the current buffer, and display a prompt.
@@ -194,6 +161,18 @@ void rdline_clear_history(struct rdline *rdl);
  */
 char *rdline_get_history_item(struct rdline *rdl, unsigned int i);
 
+/**
+ * Get maximum history buffer size.
+ */
+__rte_experimental
+size_t rdline_get_history_buffer_size(struct rdline *rdl);
+
+/**
+ * Get the opaque pointer supplied on struct rdline creation.
+ */
+__rte_experimental
+void *rdline_get_opaque(struct rdline *rdl);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/cmdline/version.map b/lib/cmdline/version.map
index 980adb4f23..b9bbb87510 100644
--- a/lib/cmdline/version.map
+++ b/lib/cmdline/version.map
@@ -57,7 +57,6 @@ DPDK_22 {
 	rdline_clear_history;
 	rdline_get_buffer;
 	rdline_get_history_item;
-	rdline_init;
 	rdline_newline;
 	rdline_quit;
 	rdline_redisplay;
@@ -73,7 +72,14 @@ DPDK_22 {
 EXPERIMENTAL {
 	global:
 
+	# added in 20.11
 	cmdline_get_rdline;
 
+	# added in 21.11
+	rdline_new;
+	rdline_free;
+	rdline_get_history_buffer_size;
+	rdline_get_opaque;
+
 	local: *;
 };
-- 
2.29.3

Re: [dpdk-dev] [PATCH v4 2/2] cmdline: make struct rdline opaque

[Stephen Hemminger]

On Tue,  5 Oct 2021 23:15:45 +0300
Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:

> +++ b/app/test/test_cmdline_lib.c
> @@ -83,18 +83,19 @@ test_cmdline_parse_fns(void)
>  static int
>  test_cmdline_rdline_fns(void)
>  {
> -	struct rdline rdl;
> +	struct rdline *rdl = NULL;

Very minor nit. The variable does not need to be initialized
since first statement assigns the value.

Re: [dpdk-dev] [PATCH v4 2/2] cmdline: make struct rdline opaque

[Olivier Matz]

Hi Dmitry,

On Tue, Oct 05, 2021 at 11:15:45PM +0300, Dmitry Kozlyuk wrote:
> Hide struct rdline definition and some RDLINE_* constants in order
> to be able to change internal buffer sizes transparently to the user.
> Add new functions:
> 
> * rdline_new(): allocate and initialize struct rdline.
>   This function replaces rdline_init() and takes an extra parameter:
>   opaque user data for the callbacks.
> * rdline_free(): deallocate struct rdline.
> * rdline_get_history_buffer_size(): for use in tests.
> * rdline_get_opaque(): to obtain user data in callback functions.
> 
> Remove rdline_init() function from library headers and export list,
> because using it requires the knowledge of sizeof(struct rdline).
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

[...]

>  /**
> - * Init fields for a struct rdline. Call this only once at the beginning
> - * of your program.
> - * \param rdl A pointer to an uninitialized struct rdline
> + * Allocate and initialize a new rdline instance.
> + *
> + * \param rdl Receives a pointer to the allocated structure.
>   * \param write_char The function used by the function to write a character
>   * \param validate A pointer to the function to execute when the
>   *                 user validates the buffer.
>   * \param complete A pointer to the function to execute when the
>   *                 user completes the buffer.
> + * \param opaque User data for use in the callbacks.
> + *
> + * \return 0 on success, negative errno-style code in failure.
>   */
> -int rdline_init(struct rdline *rdl,
> -		 rdline_write_char_t *write_char,
> -		 rdline_validate_t *validate,
> -		 rdline_complete_t *complete);
> +__rte_experimental
> +struct rdline *rdline_new(rdline_write_char_t *write_char,
> +			  rdline_validate_t *validate,
> +			  rdline_complete_t *complete,
> +			  void *opaque);

The API documentation was not updated after the v4 changes.

Apart from this, LGTM, you can directly add my ack in the next version.

Thanks,
Olivier

[dpdk-dev] [PATCH v5 0/2] cmdline: reduce ABI

[Dmitry Kozlyuk]

Hide struct cmdline following the deprecation notice.
Hide struct rdline following the v1 discussion.

v5: fix API documentation (Olivier),
    remove useless NULL assignment (Stephen).
v4: rdline_create -> rdline_new, restore empty line (Olivier).
v3: add experimental tags and releae notes for rdline.
v2: also hide struct rdline (David, Olivier).

Dmitry Kozlyuk (2):
  cmdline: make struct cmdline opaque
  cmdline: make struct rdline opaque

 app/test-cmdline/commands.c            |  2 +-
 app/test/test_cmdline_lib.c            | 22 ++++---
 doc/guides/rel_notes/deprecation.rst   |  4 --
 doc/guides/rel_notes/release_21_11.rst |  5 ++
 lib/cmdline/cmdline.c                  |  3 +-
 lib/cmdline/cmdline.h                  | 19 ------
 lib/cmdline/cmdline_private.h          | 57 ++++++++++++++++-
 lib/cmdline/cmdline_rdline.c           | 43 ++++++++++++-
 lib/cmdline/cmdline_rdline.h           | 86 ++++++++++----------------
 lib/cmdline/version.map                |  8 ++-
 10 files changed, 156 insertions(+), 93 deletions(-)

-- 
2.29.3

[dpdk-dev] [PATCH v5 1/2] cmdline: make struct cmdline opaque

[Dmitry Kozlyuk]

Remove the definition of `struct cmdline` from public header.
Deprecation notice:
https://mails.dpdk.org/archives/dev/2020-September/183310.html

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: David Marchand <david.marchand@redhat.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
---
 doc/guides/rel_notes/deprecation.rst   |  4 ----
 doc/guides/rel_notes/release_21_11.rst |  2 ++
 lib/cmdline/cmdline.h                  | 19 -------------------
 lib/cmdline/cmdline_private.h          |  8 +++++++-
 4 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 76a4abfd6b..a404276fa2 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -275,10 +275,6 @@ Deprecation Notices
 * metrics: The function ``rte_metrics_init`` will have a non-void return
   in order to notify errors instead of calling ``rte_exit``.
 
-* cmdline: ``cmdline`` structure will be made opaque to hide platform-specific
-  content. On Linux and FreeBSD, supported prior to DPDK 20.11,
-  original structure will be kept until DPDK 21.11.
-
 * security: The functions ``rte_security_set_pkt_metadata`` and
   ``rte_security_get_userdata`` will be made inline functions and additional
   flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index b55900936d..18377e5813 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -101,6 +101,8 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* cmdline: Made ``cmdline`` structure definition hidden on Linux and FreeBSD.
+
 
 ABI Changes
 -----------
diff --git a/lib/cmdline/cmdline.h b/lib/cmdline/cmdline.h
index c29762ddae..96674dfda2 100644
--- a/lib/cmdline/cmdline.h
+++ b/lib/cmdline/cmdline.h
@@ -7,10 +7,6 @@
 #ifndef _CMDLINE_H_
 #define _CMDLINE_H_
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-#include <termios.h>
-#endif
-
 #include <rte_common.h>
 #include <rte_compat.h>
 
@@ -27,23 +23,8 @@
 extern "C" {
 #endif
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-
-struct cmdline {
-	int s_in;
-	int s_out;
-	cmdline_parse_ctx_t *ctx;
-	struct rdline rdl;
-	char prompt[RDLINE_PROMPT_SIZE];
-	struct termios oldterm;
-};
-
-#else
-
 struct cmdline;
 
-#endif /* RTE_EXEC_ENV_WINDOWS */
-
 struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out);
 void cmdline_set_prompt(struct cmdline *cl, const char *prompt);
 void cmdline_free(struct cmdline *cl);
diff --git a/lib/cmdline/cmdline_private.h b/lib/cmdline/cmdline_private.h
index a87c45275c..2e93674c66 100644
--- a/lib/cmdline/cmdline_private.h
+++ b/lib/cmdline/cmdline_private.h
@@ -11,6 +11,8 @@
 #include <rte_os_shim.h>
 #ifdef RTE_EXEC_ENV_WINDOWS
 #include <rte_windows.h>
+#else
+#include <termios.h>
 #endif
 
 #include <cmdline.h>
@@ -22,6 +24,7 @@ struct terminal {
 	int is_console_input;
 	int is_console_output;
 };
+#endif
 
 struct cmdline {
 	int s_in;
@@ -29,11 +32,14 @@ struct cmdline {
 	cmdline_parse_ctx_t *ctx;
 	struct rdline rdl;
 	char prompt[RDLINE_PROMPT_SIZE];
+#ifdef RTE_EXEC_ENV_WINDOWS
 	struct terminal oldterm;
 	char repeated_char;
 	WORD repeat_count;
-};
+#else
+	struct termios oldterm;
 #endif
+};
 
 /* Disable buffering and echoing, save previous settings to oldterm. */
 void terminal_adjust(struct cmdline *cl);
-- 
2.29.3

Re: [dpdk-dev] [PATCH v5 1/2] cmdline: make struct cmdline opaque

[Narcisa Ana Maria Vasile]

On Fri, Oct 08, 2021 at 01:10:27AM +0300, Dmitry Kozlyuk wrote:
> Remove the definition of `struct cmdline` from public header.
> Deprecation notice:
> https://mails.dpdk.org/archives/dev/2020-September/183310.html
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Acked-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> ---
Acked-by: Narcisa Vasile <navasile@linux.microsoft.com>

[dpdk-dev] [PATCH v5 2/2] cmdline: make struct rdline opaque

[Dmitry Kozlyuk]

Hide struct rdline definition and some RDLINE_* constants in order
to be able to change internal buffer sizes transparently to the user.
Add new functions:

* rdline_new(): allocate and initialize struct rdline.
  This function replaces rdline_init() and takes an extra parameter:
  opaque user data for the callbacks.
* rdline_free(): deallocate struct rdline.
* rdline_get_history_buffer_size(): for use in tests.
* rdline_get_opaque(): to obtain user data in callback functions.

Remove rdline_init() function from library headers and export list,
because using it requires the knowledge of sizeof(struct rdline).

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test-cmdline/commands.c            |  2 +-
 app/test/test_cmdline_lib.c            | 22 ++++---
 doc/guides/rel_notes/release_21_11.rst |  3 +
 lib/cmdline/cmdline.c                  |  3 +-
 lib/cmdline/cmdline_private.h          | 49 +++++++++++++++
 lib/cmdline/cmdline_rdline.c           | 43 ++++++++++++-
 lib/cmdline/cmdline_rdline.h           | 86 ++++++++++----------------
 lib/cmdline/version.map                |  8 ++-
 8 files changed, 147 insertions(+), 69 deletions(-)

diff --git a/app/test-cmdline/commands.c b/app/test-cmdline/commands.c
index d732976f08..a13e1d1afd 100644
--- a/app/test-cmdline/commands.c
+++ b/app/test-cmdline/commands.c
@@ -297,7 +297,7 @@ cmd_get_history_bufsize_parsed(__rte_unused void *parsed_result,
 	struct rdline *rdl = cmdline_get_rdline(cl);
 
 	cmdline_printf(cl, "History buffer size: %zu\n",
-			sizeof(rdl->history_buf));
+			rdline_get_history_buffer_size(rdl));
 }
 
 cmdline_parse_token_string_t cmd_get_history_bufsize_tok =
diff --git a/app/test/test_cmdline_lib.c b/app/test/test_cmdline_lib.c
index d5a09b4541..054ebf5e9d 100644
--- a/app/test/test_cmdline_lib.c
+++ b/app/test/test_cmdline_lib.c
@@ -83,18 +83,19 @@ test_cmdline_parse_fns(void)
 static int
 test_cmdline_rdline_fns(void)
 {
-	struct rdline rdl;
+	struct rdline *rdl;
 	rdline_write_char_t *wc = &cmdline_write_char;
 	rdline_validate_t *v = &valid_buffer;
 	rdline_complete_t *c = &complete_buffer;
 
-	if (rdline_init(NULL, wc, v, c) >= 0)
+	rdl = rdline_new(NULL, v, c, NULL);
+	if (rdl != NULL)
 		goto error;
-	if (rdline_init(&rdl, NULL, v, c) >= 0)
+	rdl = rdline_new(wc, NULL, c, NULL);
+	if (rdl != NULL)
 		goto error;
-	if (rdline_init(&rdl, wc, NULL, c) >= 0)
-		goto error;
-	if (rdline_init(&rdl, wc, v, NULL) >= 0)
+	rdl = rdline_new(wc, v, NULL, NULL);
+	if (rdl != NULL)
 		goto error;
 	if (rdline_char_in(NULL, 0) >= 0)
 		goto error;
@@ -102,25 +103,30 @@ test_cmdline_rdline_fns(void)
 		goto error;
 	if (rdline_add_history(NULL, "history") >= 0)
 		goto error;
-	if (rdline_add_history(&rdl, NULL) >= 0)
+	if (rdline_add_history(rdl, NULL) >= 0)
 		goto error;
 	if (rdline_get_history_item(NULL, 0) != NULL)
 		goto error;
 
 	/* void functions */
+	rdline_get_history_buffer_size(NULL);
+	rdline_get_opaque(NULL);
 	rdline_newline(NULL, "prompt");
-	rdline_newline(&rdl, NULL);
+	rdline_newline(rdl, NULL);
 	rdline_stop(NULL);
 	rdline_quit(NULL);
 	rdline_restart(NULL);
 	rdline_redisplay(NULL);
 	rdline_reset(NULL);
 	rdline_clear_history(NULL);
+	rdline_free(NULL);
 
+	rdline_free(rdl);
 	return 0;
 
 error:
 	printf("Error: function accepted null parameter!\n");
+	rdline_free(rdl);
 	return -1;
 }
 
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index 18377e5813..af11f4a656 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -103,6 +103,9 @@ API Changes
 
 * cmdline: Made ``cmdline`` structure definition hidden on Linux and FreeBSD.
 
+* cmdline: Made ``rdline`` structure definition hidden. Functions are added
+  to dynamically allocate and free it, and to access user data in callbacks.
+
 
 ABI Changes
 -----------
diff --git a/lib/cmdline/cmdline.c b/lib/cmdline/cmdline.c
index a176d15130..8f1854cb0b 100644
--- a/lib/cmdline/cmdline.c
+++ b/lib/cmdline/cmdline.c
@@ -85,13 +85,12 @@ cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_in, int s_out)
 	cl->ctx = ctx;
 
 	ret = rdline_init(&cl->rdl, cmdline_write_char, cmdline_valid_buffer,
-			cmdline_complete_buffer);
+			cmdline_complete_buffer, cl);
 	if (ret != 0) {
 		free(cl);
 		return NULL;
 	}
 
-	cl->rdl.opaque = cl;
 	cmdline_set_prompt(cl, prompt);
 	rdline_newline(&cl->rdl, cl->prompt);
 
diff --git a/lib/cmdline/cmdline_private.h b/lib/cmdline/cmdline_private.h
index 2e93674c66..c2e906d8de 100644
--- a/lib/cmdline/cmdline_private.h
+++ b/lib/cmdline/cmdline_private.h
@@ -17,6 +17,49 @@
 
 #include <cmdline.h>
 
+#define RDLINE_BUF_SIZE 512
+#define RDLINE_PROMPT_SIZE  32
+#define RDLINE_VT100_BUF_SIZE  8
+#define RDLINE_HISTORY_BUF_SIZE BUFSIZ
+#define RDLINE_HISTORY_MAX_LINE 64
+
+enum rdline_status {
+	RDLINE_INIT,
+	RDLINE_RUNNING,
+	RDLINE_EXITED
+};
+
+struct rdline {
+	enum rdline_status status;
+	/* rdline bufs */
+	struct cirbuf left;
+	struct cirbuf right;
+	char left_buf[RDLINE_BUF_SIZE+2]; /* reserve 2 chars for the \n\0 */
+	char right_buf[RDLINE_BUF_SIZE];
+
+	char prompt[RDLINE_PROMPT_SIZE];
+	unsigned int prompt_size;
+
+	char kill_buf[RDLINE_BUF_SIZE];
+	unsigned int kill_size;
+
+	/* history */
+	struct cirbuf history;
+	char history_buf[RDLINE_HISTORY_BUF_SIZE];
+	int history_cur_line;
+
+	/* callbacks and func pointers */
+	rdline_write_char_t *write_char;
+	rdline_validate_t *validate;
+	rdline_complete_t *complete;
+
+	/* vt100 parser */
+	struct cmdline_vt100 vt100;
+
+	/* opaque pointer */
+	void *opaque;
+};
+
 #ifdef RTE_EXEC_ENV_WINDOWS
 struct terminal {
 	DWORD input_mode;
@@ -57,4 +100,10 @@ ssize_t cmdline_read_char(struct cmdline *cl, char *c);
 __rte_format_printf(2, 0)
 int cmdline_vdprintf(int fd, const char *format, va_list op);
 
+int rdline_init(struct rdline *rdl,
+		rdline_write_char_t *write_char,
+		rdline_validate_t *validate,
+		rdline_complete_t *complete,
+		void *opaque);
+
 #endif
diff --git a/lib/cmdline/cmdline_rdline.c b/lib/cmdline/cmdline_rdline.c
index 2cb53e38f2..d92b1cda53 100644
--- a/lib/cmdline/cmdline_rdline.c
+++ b/lib/cmdline/cmdline_rdline.c
@@ -13,6 +13,7 @@
 #include <ctype.h>
 
 #include "cmdline_cirbuf.h"
+#include "cmdline_private.h"
 #include "cmdline_rdline.h"
 
 static void rdline_puts(struct rdline *rdl, const char *buf);
@@ -37,9 +38,10 @@ isblank2(char c)
 
 int
 rdline_init(struct rdline *rdl,
-		 rdline_write_char_t *write_char,
-		 rdline_validate_t *validate,
-		 rdline_complete_t *complete)
+	    rdline_write_char_t *write_char,
+	    rdline_validate_t *validate,
+	    rdline_complete_t *complete,
+	    void *opaque)
 {
 	if (!rdl || !write_char || !validate || !complete)
 		return -EINVAL;
@@ -47,10 +49,33 @@ rdline_init(struct rdline *rdl,
 	rdl->validate = validate;
 	rdl->complete = complete;
 	rdl->write_char = write_char;
+	rdl->opaque = opaque;
 	rdl->status = RDLINE_INIT;
 	return cirbuf_init(&rdl->history, rdl->history_buf, 0, RDLINE_HISTORY_BUF_SIZE);
 }
 
+struct rdline *
+rdline_new(rdline_write_char_t *write_char,
+	   rdline_validate_t *validate,
+	   rdline_complete_t *complete,
+	   void *opaque)
+{
+	struct rdline *rdl;
+
+	rdl = malloc(sizeof(*rdl));
+	if (rdline_init(rdl, write_char, validate, complete, opaque) < 0) {
+		free(rdl);
+		rdl = NULL;
+	}
+	return rdl;
+}
+
+void
+rdline_free(struct rdline *rdl)
+{
+	free(rdl);
+}
+
 void
 rdline_newline(struct rdline *rdl, const char *prompt)
 {
@@ -564,6 +589,18 @@ rdline_get_history_item(struct rdline * rdl, unsigned int idx)
 	return NULL;
 }
 
+size_t
+rdline_get_history_buffer_size(struct rdline *rdl)
+{
+	return sizeof(rdl->history_buf);
+}
+
+void *
+rdline_get_opaque(struct rdline *rdl)
+{
+	return rdl != NULL ? rdl->opaque : NULL;
+}
+
 int
 rdline_add_history(struct rdline * rdl, const char * buf)
 {
diff --git a/lib/cmdline/cmdline_rdline.h b/lib/cmdline/cmdline_rdline.h
index d2170293de..1b4cc7ce57 100644
--- a/lib/cmdline/cmdline_rdline.h
+++ b/lib/cmdline/cmdline_rdline.h
@@ -10,9 +10,7 @@
 /**
  * This file is a small equivalent to the GNU readline library, but it
  * was originally designed for small systems, like Atmel AVR
- * microcontrollers (8 bits). Indeed, we don't use any malloc that is
- * sometimes not implemented (or just not recommended) on such
- * systems.
+ * microcontrollers (8 bits). It only uses malloc() on object creation.
  *
  * Obviously, it does not support as many things as the GNU readline,
  * but at least it supports some interesting features like a kill
@@ -31,6 +29,7 @@
  */
 
 #include <stdio.h>
+#include <rte_compat.h>
 #include <cmdline_cirbuf.h>
 #include <cmdline_vt100.h>
 
@@ -38,19 +37,6 @@
 extern "C" {
 #endif
 
-/* configuration */
-#define RDLINE_BUF_SIZE 512
-#define RDLINE_PROMPT_SIZE  32
-#define RDLINE_VT100_BUF_SIZE  8
-#define RDLINE_HISTORY_BUF_SIZE BUFSIZ
-#define RDLINE_HISTORY_MAX_LINE 64
-
-enum rdline_status {
-	RDLINE_INIT,
-	RDLINE_RUNNING,
-	RDLINE_EXITED
-};
-
 struct rdline;
 
 typedef int (rdline_write_char_t)(struct rdline *rdl, char);
@@ -60,52 +46,32 @@ typedef int (rdline_complete_t)(struct rdline *rdl, const char *buf,
 				char *dstbuf, unsigned int dstsize,
 				int *state);
 
-struct rdline {
-	enum rdline_status status;
-	/* rdline bufs */
-	struct cirbuf left;
-	struct cirbuf right;
-	char left_buf[RDLINE_BUF_SIZE+2]; /* reserve 2 chars for the \n\0 */
-	char right_buf[RDLINE_BUF_SIZE];
-
-	char prompt[RDLINE_PROMPT_SIZE];
-	unsigned int prompt_size;
-
-	char kill_buf[RDLINE_BUF_SIZE];
-	unsigned int kill_size;
-
-	/* history */
-	struct cirbuf history;
-	char history_buf[RDLINE_HISTORY_BUF_SIZE];
-	int history_cur_line;
-
-	/* callbacks and func pointers */
-	rdline_write_char_t *write_char;
-	rdline_validate_t *validate;
-	rdline_complete_t *complete;
-
-	/* vt100 parser */
-	struct cmdline_vt100 vt100;
-
-	/* opaque pointer */
-	void *opaque;
-};
-
 /**
- * Init fields for a struct rdline. Call this only once at the beginning
- * of your program.
- * \param rdl A pointer to an uninitialized struct rdline
+ * Allocate and initialize a new rdline instance.
+ *
  * \param write_char The function used by the function to write a character
  * \param validate A pointer to the function to execute when the
  *                 user validates the buffer.
  * \param complete A pointer to the function to execute when the
  *                 user completes the buffer.
+ * \param opaque User data for use in the callbacks.
+ *
+ * \return New rdline object on success, NULL on failure.
  */
-int rdline_init(struct rdline *rdl,
-		 rdline_write_char_t *write_char,
-		 rdline_validate_t *validate,
-		 rdline_complete_t *complete);
+__rte_experimental
+struct rdline *rdline_new(rdline_write_char_t *write_char,
+			  rdline_validate_t *validate,
+			  rdline_complete_t *complete,
+			  void *opaque);
 
+/**
+ * Free an rdline instance.
+ *
+ * \param rdl A pointer to an initialized struct rdline.
+ *            If NULL, this function is a no-op.
+ */
+__rte_experimental
+void rdline_free(struct rdline *rdl);
 
 /**
  * Init the current buffer, and display a prompt.
@@ -194,6 +160,18 @@ void rdline_clear_history(struct rdline *rdl);
  */
 char *rdline_get_history_item(struct rdline *rdl, unsigned int i);
 
+/**
+ * Get maximum history buffer size.
+ */
+__rte_experimental
+size_t rdline_get_history_buffer_size(struct rdline *rdl);
+
+/**
+ * Get the opaque pointer supplied on struct rdline creation.
+ */
+__rte_experimental
+void *rdline_get_opaque(struct rdline *rdl);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/cmdline/version.map b/lib/cmdline/version.map
index 980adb4f23..b9bbb87510 100644
--- a/lib/cmdline/version.map
+++ b/lib/cmdline/version.map
@@ -57,7 +57,6 @@ DPDK_22 {
 	rdline_clear_history;
 	rdline_get_buffer;
 	rdline_get_history_item;
-	rdline_init;
 	rdline_newline;
 	rdline_quit;
 	rdline_redisplay;
@@ -73,7 +72,14 @@ DPDK_22 {
 EXPERIMENTAL {
 	global:
 
+	# added in 20.11
 	cmdline_get_rdline;
 
+	# added in 21.11
+	rdline_new;
+	rdline_free;
+	rdline_get_history_buffer_size;
+	rdline_get_opaque;
+
 	local: *;
 };
-- 
2.29.3

Re: [dpdk-dev] [PATCH v5 2/2] cmdline: make struct rdline opaque

[Narcisa Ana Maria Vasile]

On Fri, Oct 08, 2021 at 01:10:28AM +0300, Dmitry Kozlyuk wrote:
> Hide struct rdline definition and some RDLINE_* constants in order
> to be able to change internal buffer sizes transparently to the user.
> Add new functions:
> 
> * rdline_new(): allocate and initialize struct rdline.
>   This function replaces rdline_init() and takes an extra parameter:
>   opaque user data for the callbacks.
> * rdline_free(): deallocate struct rdline.
> * rdline_get_history_buffer_size(): for use in tests.
> * rdline_get_opaque(): to obtain user data in callback functions.
> 
> Remove rdline_init() function from library headers and export list,
> because using it requires the knowledge of sizeof(struct rdline).
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> ---

Acked-by: Narcisa Vasile <navasile@linux.microsoft.com>

Re: [dpdk-dev] [PATCH v5 0/2] cmdline: reduce ABI

[Thomas Monjalon]

> Dmitry Kozlyuk (2):
>   cmdline: make struct cmdline opaque
>   cmdline: make struct rdline opaque

Applied, thanks.