Re: [PATCH 2/3] conf: Don't print usage via the logging subsystem

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 29 May 2024 19:04:04 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> The message from usage() when given invalid options, or the -h / --help
> option is currently printed by many calls to the info() function, also
> used for runtime logging of informational messages.
> 
> That isn't useful: the usage message should always go to the terminal
> (stdout or stderr), never syslog or a logfile.  It should never be
> filtered by priority.  Really the only thing using the common logging
> functions does is give more opportunities for something to go wrong.
> 
> Replace all the info() calls with direct fprintf() calls.  This does mean
> manually adding "\n" to each message.  A little messy, but worth it for the
> simplicity in other dimensions.

Yes, definitely less messy than the existing implementation, I just
wonder:

> Link: https://bugs.passt.top/show_bug.cgi?id=90
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  conf.c | 318 ++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 159 insertions(+), 159 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index f2a92574..31f5b197 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -706,193 +706,194 @@ static unsigned int conf_ip6(unsigned int ifi,
>  /**
>   * usage() - Print usage, exit with given status code
>   * @name:	Executable name
> + * @f:		Stream to print usage info to
>   * @status:	Status code for exit()
>   */
> -static void usage(const char *name, int status)
> +static void usage(const char *name, FILE *f, int status)
>  {
>  	if (strstr(name, "pasta")) {
> -		info("Usage: %s [OPTION]... [COMMAND] [ARGS]...", name);
> -		info("       %s [OPTION]... PID", name);
> -		info("       %s [OPTION]... --netns [PATH|NAME]", name);
> -		info("");
> -		info("Without PID or --netns, run the given command or a");
> -		info("default shell in a new network and user namespace, and");
> -		info("connect it via pasta.");
> +		fprintf(f, "Usage: %s [OPTION]... [COMMAND] [ARGS]...\n", name);
> +		fprintf(f, "       %s [OPTION]... PID\n", name);
> +		fprintf(f, "       %s [OPTION]... --netns [PATH|NAME]\n", name);
> +		fprintf(f, "\n");
> +		fprintf(f, "Without PID or --netns, run the given command or a\n");
> +		fprintf(f, "default shell in a new network and user namespace, and\n");
> +		fprintf(f, "connect it via pasta.\n");

I haven't checked how it looks like in the end, but in most of this
function, we use fprintf() without arguments after the format, and we
need explicit newlines anyway, so, what if we concatenate the whole
output, say:

		fprintf(f,
			"Without PID or --netns, run the given command or a\n"
			"default shell in a new network and user namespace, and\n"
			"connect it via pasta.\n"
			);

?

I used separate info() calls (or whatever they were) in the past
just because of the convenient newlines.

-- 
Stefano