Re: [PATCH 3/4] tools/bootconfig: Fix to return 0 if succeeded to show the bootconfig

[Steven Rostedt <rostedt@goodmis.org>]

On Sat, 13 Jun 2020 00:23:35 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Fix bootconfig to return 0 if succeeded to show the bootconfig
> in initrd. Without this fix, "bootconfig INITRD" command
> returns !0 even if the command succeeded to show the bootconfig.
> 
> Fixes: 950313ebf79c ("tools: bootconfig: Add bootconfig command")
> Cc: stable@vger.kernel.org
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/bootconfig/main.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
> index 21896a6675fd..ff2cc9520e10 100644
> --- a/tools/bootconfig/main.c
> +++ b/tools/bootconfig/main.c
> @@ -209,8 +209,10 @@ int show_xbc(const char *path)
>  	ret = load_xbc_from_initrd(fd, &buf);
>  	if (ret < 0)
>  		pr_err("Failed to load a boot config from initrd: %d\n", ret);
> -	else
> +	else {
>  		xbc_show_compact_tree();
> +		ret = 0;
> +	}

Usually for the above, I think goto is cleaner. As it is strange to
have a successful case as the "else" condition. Not to mention, if you
add brackets for one side of the else, it is usually recommended to add
them for the other side.

But in this case I think it would read better to have:


	if (ret < 0) {
		pr_err(...);
		goto out;
	}
	xbc_show_compact_tree();
	ret = 0;
out:
	
>  
>  	close(fd);
>  	free(buf);

-- Steve