Re: [Unbound-core] proposed diff: vcli_loop()

[Juli M. <jma...@Fr...>]

* De: Jose Nazario <jo...@mo...> [ Data: 2002-11-19 ]
	[ Subjecte: [Unbound-core] proposed diff: vcli_loop() ]
> this patch adds the library function vcli_loop(), which uses a variable
> prompt. it is documented in the cli.3 manpage if you would like to see. i
> haven't been able to get it to work, mainly because of a problem revealed
> by the compiler:
> 
> cli.c:119: warning: passing arg 2 of `strlcpy' makes pointer from integer
> without a cast
> 
> have a look, please fix up, but more importantly is the interface decent
> and clean?
> 
> this patch also fixes a minor misuse of prompt vs cli_prompt in
> cli_loop():
> 
> -                       fprintf(streamout, "%s", prompt);       /* XXX */
> +                       fprintf(streamout, "%s", cli_prompt);   /* XXX */
> 
> basically we truncated (via strlcpy()) for nothing, this uses what we
> intended to use.

I don't understand why we strlcpy?  Also, strlcpy was used wrong, I think,
so I added an XXX.

> ? test/test2.c
> Index: cli.c
> ===================================================================
> RCS file: /cvsroot/unbound/unbound/lib/cli/cli.c,v
> retrieving revision 1.5
> diff -u -r1.5 cli.c
> --- cli.c	19 Nov 2002 05:30:54 -0000	1.5
> +++ cli.c	19 Nov 2002 18:08:03 -0000
> @@ -68,7 +68,7 @@
>  		char            *input, *forward[MAX_TOKENS];
>  
>  		if (streamout != NULL)
> -			fprintf(streamout, "%s", prompt);	/* XXX */
> +			fprintf(streamout, "%s", cli_prompt);	/* XXX */
>  
>  		if ((input = fparseln(streamin, NULL, NULL, NULL, 0)) == NULL) {
>  			break;
> @@ -89,4 +89,54 @@
>  		fclose(streamout);
>  	fclose(streamin);
>  	return (0);
> +}
> +
> +int
> +vcli_loop(int fdin, int fdout, prompt_handler p_callback, cli_handler callback) {
> +	char            *p;
> +	FILE            *streamin;
> +	FILE            *streamout = NULL;
> +	size_t           len;
> +	
> +	if ((streamin = fdopen(fdin, "r")) == NULL) {
> +		warn("Failed to open specified file descriptor");
> +		return(errno);
> +	}
> +
> +	if (fdout > -1) 
> +		if ((streamout = fdopen(fdout, "w+")) == NULL) {
> +			warn("Failed to open specific file descriptor");
> +			return(errno);
> +		}
> +
> +	for (;;) {
> +		int 		i = 0;
> +		char	       *input, *forward[MAX_TOKENS];
> +		char           *cli_prompt[MAX_PROMPT_SIZE];

No.  You don't want an array of pointers to characters for cli_prompt, you
want an array of characters.

> +		if (streamout != NULL) {
> +			strlcpy((char *)cli_prompt, p_callback(),
> +			         MAX_PROMPT_SIZE);
> +			fprintf(streamout, "%s", cli_prompt);
> +		}
> +
> +		if ((input = fparseln(streamin, NULL, NULL, NULL, 0)) == NULL) {
> +			break;
> +		}
> +
> +		for (; i < MAX_TOKENS-1 &&
> +		    (forward[i] = strsep(&input, " \t")) != NULL;)
> +			if (*forward[i] != '\0')
> +				i++;
> +
> +		if (i > 0)
> +			if (callback (i, forward) < 0)
> +				return (-1);
> +		free(input);
> +	}
> +
> +	if (streamout != NULL)
> +		fclose(streamout);
> +	fclose(streamin);
> +	return(0);
>  }
> Index: include/cli.h
> ===================================================================
> RCS file: /cvsroot/unbound/unbound/lib/cli/include/cli.h,v
> retrieving revision 1.3
> diff -u -r1.3 cli.h
> --- include/cli.h	19 Nov 2002 04:43:39 -0000	1.3
> +++ include/cli.h	19 Nov 2002 18:08:03 -0000
> @@ -30,6 +30,8 @@
>  
>  typedef int     (*cli_handler)(int, char *[]);
>  int             cli_loop(int, int, const char *, cli_handler);
> +typedef char    (*prompt_handler)();

No.

typedef	char	*(*prompt_handler)(void);

It might make sense to use _t for all these opaque types, otherwise it's
confusing, and prevents the use of obvious things like:

	prompt_handler_t prompt_handler

> +int		vcli_loop(int, int, prompt_handler, cli_handler);
>  
>  #define MAX_PROMPT_SIZE		255
>  #define MAX_TOKENS 		512
> Index: man/cli.3
> ===================================================================
> RCS file: /cvsroot/unbound/unbound/lib/cli/man/cli.3,v
> retrieving revision 1.4
> diff -u -r1.4 cli.3
> --- man/cli.3	19 Nov 2002 04:43:39 -0000	1.4
> +++ man/cli.3	19 Nov 2002 18:08:03 -0000
> @@ -13,6 +13,9 @@
>  .Fd #include <cli.h>
>  .Ft int
>  .Fn cli_loop "int fdin" "int fdout" "const char *prompt" "cli_handler callback"
> +.Ft int
> +.Fn vcli_loop "int fdin" "int fdout" "prompt_handler p_callback" "cli_handler callback"
> +.\"

Below.

>  .Sh DESCRIPTION
>  .Nm
>  provides a simple API to develop command line based utilities.
> @@ -37,12 +40,23 @@
>  .Fa fdout
>  to -1 causes the output from cli_loop to be silenced. This is designed for
>  non interactive shells.
> +.Pp
> +.Fn vcli_loop
> +is like 
> +.Fn cli_loop
> +only the displayed prompt is evaluated by
> +.Fa p_callback
> +each time to determine its format. This is useful for systems where the 
> +prompt changes in response to some environmental factor, such as priviledge
> +level or mode.
> +.\"

Below.

>  .Sh BUGS
>  To link completely against 
>  .Nm
>  you will also have to link against the
>  .Fa libutil
>  library. No other bugs known, aside from the fact that this is a dumb library.
> +.\"

Why the comments before each section?

>  .Sh AUTHORS
>  Jose Nazario
>  .Aq jo...@mo...

Thanks!
juli.
-- 
Juli Mallett <jma...@Fr...>
OpenDarwin, Mono, FreeBSD Developer.
ircd-hybrid Developer, EFnet addict.
FreeBSD on MIPS-Anything on FreeBSD.