diff mbox series

[v2,01/11] multipathd: replace libreadline with getline()

Message ID 20220822204119.20719-2-mwilck@suse.com (mailing list archive)
State Superseded, archived
Delegated to: christophe varoqui
Headers show
Series Split libmultipath and libmpathutil | expand

Commit Message

Martin Wilck Aug. 22, 2022, 8:41 p.m. UTC
From: Hannes Reinecke <hare@suse.de>

libreadline changed the license to be incompatible with multipath-tools
usage, so replace it with a simple getline().

mwilck: Make this the default option via Makefile.inc; it is used if
READLINE is unset. Compiling with READLINE=libreadline or READLINE=libedit
remains possible.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 Makefile.inc        |  4 ++--
 multipathd/cli.c    |  2 ++
 multipathd/uxclnt.c | 50 ++++++++++++++++++++++++++++++---------------
 3 files changed, 38 insertions(+), 18 deletions(-)

Comments

Benjamin Marzinski Aug. 29, 2022, 4:55 p.m. UTC | #1
On Mon, Aug 22, 2022 at 10:41:09PM +0200, mwilck@suse.com wrote:
> From: Hannes Reinecke <hare@suse.de>
> 
> libreadline changed the license to be incompatible with multipath-tools
> usage, so replace it with a simple getline().
> 
> mwilck: Make this the default option via Makefile.inc; it is used if
> READLINE is unset. Compiling with READLINE=libreadline or READLINE=libedit
> remains possible.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  Makefile.inc        |  4 ++--
>  multipathd/cli.c    |  2 ++
>  multipathd/uxclnt.c | 50 ++++++++++++++++++++++++++++++---------------
>  3 files changed, 38 insertions(+), 18 deletions(-)
> 
> diff --git a/Makefile.inc b/Makefile.inc
> index ad7afd0..0653d21 100644
> --- a/Makefile.inc
> +++ b/Makefile.inc
> @@ -9,10 +9,10 @@
>  # Uncomment to disable dmevents polling support
>  # ENABLE_DMEVENTS_POLL = 0
>  #
> -# Readline library to use, libedit or libreadline
> +# Readline library to use, libedit, libreadline, or empty
>  # Caution: Using libreadline may make the multipathd binary undistributable,
>  # see https://github.com/opensvc/multipath-tools/issues/36
> -READLINE = libedit
> +READLINE := 

Trailing whitespace nitpick.

>  
>  # List of scsi device handler modules to load on boot, e.g.
>  # SCSI_DH_MODULES_PRELOAD := scsi_dh_alua scsi_dh_rdac
> diff --git a/multipathd/cli.c b/multipathd/cli.c
> index fa482a6..cc56950 100644
> --- a/multipathd/cli.c
> +++ b/multipathd/cli.c
> @@ -459,6 +459,7 @@ void cli_exit(void)
>  	keys = NULL;
>  }
>  
> +#if defined(USE_LIBREADLINE) || defined(USE_LIBEDIT)
>  static int
>  key_match_fingerprint (struct key * kw, uint64_t fp)
>  {
> @@ -564,3 +565,4 @@ key_generator (const char * str, int state)
>  	 */
>  	return ((char *)NULL);
>  }
> +#endif

It's kind of odd to not define key_generator(), but leave it defined in
cli.h if no library is defined.

> diff --git a/multipathd/uxclnt.c b/multipathd/uxclnt.c
> index 251e7d7..b817bea 100644
> --- a/multipathd/uxclnt.c
> +++ b/multipathd/uxclnt.c
> @@ -30,6 +30,7 @@
>  #include "defaults.h"
>  
>  #include "vector.h"
> +#include "util.h"
>  #include "cli.h"
>  #include "uxclnt.h"
>  
> @@ -77,35 +78,52 @@ static int need_quit(char *str, size_t len)
>   */
>  static void process(int fd, unsigned int timeout)
>  {
> -	char *line;
> -	char *reply;
> -	int ret;
>  
> -	cli_init();
> +#if defined(USE_LIBREADLINE) || defined(USE_LIBEDIT)
>  	rl_readline_name = "multipathd";
>  	rl_completion_entry_function = key_generator;
> -	while ((line = readline("multipathd> "))) {
> -		size_t llen = strlen(line);
> +#endif
>  
> -		if (!llen) {
> -			free(line);
> +	cli_init();
> +	for(;;)
> +	{
> +		char *line __attribute__((cleanup(cleanup_charp))) = NULL;
> +		char *reply __attribute__((cleanup(cleanup_charp))) = NULL;
> +		ssize_t llen;
> +		int ret;
> +
> +#if defined(USE_LIBREADLINE) || defined(USE_LIBEDIT)
> +		line = readline("multipathd> ");
> +		if (!line)
> +			break;
> +		llen = strlen(line);
> +		if (!llen)
>  			continue;
> +#else
> +		size_t lsize = 0;
> +
> +		fputs("multipathd> ", stdout);
> +		errno = 0;
> +		llen = getline(&line, &lsize, stdin);
> +		if (llen == -1) {
> +			if (errno != 0)
> +				fprintf(stderr, "Error in getline: %m");
> +			break;
>  		}
> +		if (!llen || !strcmp(line, "\n"))
> +			continue;
> +#endif
>  
>  		if (need_quit(line, llen))
>  			break;
>  
> -		if (send_packet(fd, line) != 0) break;
> +		if (send_packet(fd, line) != 0)
> +			break;
>  		ret = recv_packet(fd, &reply, timeout);
> -		if (ret != 0) break;
> +		if (ret != 0)
> +			break;
>  
>  		print_reply(reply);
> -
> -		if (line && *line)
> -			add_history(line);
> -
> -		free(line);
> -		free(reply);

Why drop add_history() support even if a library is defined?

-Ben

>  	}
>  }
>  
> -- 
> 2.37.1
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
diff mbox series

Patch

diff --git a/Makefile.inc b/Makefile.inc
index ad7afd0..0653d21 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -9,10 +9,10 @@ 
 # Uncomment to disable dmevents polling support
 # ENABLE_DMEVENTS_POLL = 0
 #
-# Readline library to use, libedit or libreadline
+# Readline library to use, libedit, libreadline, or empty
 # Caution: Using libreadline may make the multipathd binary undistributable,
 # see https://github.com/opensvc/multipath-tools/issues/36
-READLINE = libedit
+READLINE := 
 
 # List of scsi device handler modules to load on boot, e.g.
 # SCSI_DH_MODULES_PRELOAD := scsi_dh_alua scsi_dh_rdac
diff --git a/multipathd/cli.c b/multipathd/cli.c
index fa482a6..cc56950 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -459,6 +459,7 @@  void cli_exit(void)
 	keys = NULL;
 }
 
+#if defined(USE_LIBREADLINE) || defined(USE_LIBEDIT)
 static int
 key_match_fingerprint (struct key * kw, uint64_t fp)
 {
@@ -564,3 +565,4 @@  key_generator (const char * str, int state)
 	 */
 	return ((char *)NULL);
 }
+#endif
diff --git a/multipathd/uxclnt.c b/multipathd/uxclnt.c
index 251e7d7..b817bea 100644
--- a/multipathd/uxclnt.c
+++ b/multipathd/uxclnt.c
@@ -30,6 +30,7 @@ 
 #include "defaults.h"
 
 #include "vector.h"
+#include "util.h"
 #include "cli.h"
 #include "uxclnt.h"
 
@@ -77,35 +78,52 @@  static int need_quit(char *str, size_t len)
  */
 static void process(int fd, unsigned int timeout)
 {
-	char *line;
-	char *reply;
-	int ret;
 
-	cli_init();
+#if defined(USE_LIBREADLINE) || defined(USE_LIBEDIT)
 	rl_readline_name = "multipathd";
 	rl_completion_entry_function = key_generator;
-	while ((line = readline("multipathd> "))) {
-		size_t llen = strlen(line);
+#endif
 
-		if (!llen) {
-			free(line);
+	cli_init();
+	for(;;)
+	{
+		char *line __attribute__((cleanup(cleanup_charp))) = NULL;
+		char *reply __attribute__((cleanup(cleanup_charp))) = NULL;
+		ssize_t llen;
+		int ret;
+
+#if defined(USE_LIBREADLINE) || defined(USE_LIBEDIT)
+		line = readline("multipathd> ");
+		if (!line)
+			break;
+		llen = strlen(line);
+		if (!llen)
 			continue;
+#else
+		size_t lsize = 0;
+
+		fputs("multipathd> ", stdout);
+		errno = 0;
+		llen = getline(&line, &lsize, stdin);
+		if (llen == -1) {
+			if (errno != 0)
+				fprintf(stderr, "Error in getline: %m");
+			break;
 		}
+		if (!llen || !strcmp(line, "\n"))
+			continue;
+#endif
 
 		if (need_quit(line, llen))
 			break;
 
-		if (send_packet(fd, line) != 0) break;
+		if (send_packet(fd, line) != 0)
+			break;
 		ret = recv_packet(fd, &reply, timeout);
-		if (ret != 0) break;
+		if (ret != 0)
+			break;
 
 		print_reply(reply);
-
-		if (line && *line)
-			add_history(line);
-
-		free(line);
-		free(reply);
 	}
 }