Messages in this thread | | | Date | Tue, 16 Jun 2020 16:53:37 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH 3/4] tools/bootconfig: Fix to return 0 if succeeded to show the bootconfig |
| |
On Mon, 15 Jun 2020 15:14:42 -0400 Steven Rostedt <rostedt@goodmis.org> wrote:
> 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:
Agreed. OK, I'll update it.
Thank you!
> > > > > close(fd); > > free(buf); > > -- Steve
-- Masami Hiramatsu <mhiramat@kernel.org>
| |