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

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

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