* [PATCH 0/4] bootconfig: Fix quote-character issue and return value @ 2020-06-12 15:23 Masami Hiramatsu 2020-06-12 15:23 ` [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value Masami Hiramatsu ` (3 more replies) 0 siblings, 4 replies; 18+ messages in thread From: Masami Hiramatsu @ 2020-06-12 15:23 UTC (permalink / raw) To: Steven Rostedt; +Cc: stable, LKML Hi Steve, I found 2 bugs in /proc/bootconfig and tools/bootconfig. - They always use double-quote to quote values. For the values which includes double-quote, it should use single-quote instead. - tools/bootconfig always returns error code if it shows the bootconfig in initrd (executed without options) This series fixes those bugs and add testcases to ensure no regressions. Thank you, --- Masami Hiramatsu (4): proc/bootconfig: Fix to use correct quotes for value tools/bootconfig: Fix to use correct quotes for value tools/bootconfig: Fix to return 0 if succeeded to show the bootconfig tools/bootconfig: Add testcase for show-command and quotes test fs/proc/bootconfig.c | 13 +++++++++---- tools/bootconfig/main.c | 18 +++++++++++------- tools/bootconfig/test-bootconfig.sh | 10 ++++++++++ 3 files changed, 30 insertions(+), 11 deletions(-) -- Masami Hiramatsu (Linaro) <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value 2020-06-12 15:23 [PATCH 0/4] bootconfig: Fix quote-character issue and return value Masami Hiramatsu @ 2020-06-12 15:23 ` Masami Hiramatsu 2020-06-12 22:42 ` Masami Hiramatsu 2020-06-15 19:11 ` Steven Rostedt 2020-06-12 15:23 ` [PATCH 2/4] tools/bootconfig: " Masami Hiramatsu ` (2 subsequent siblings) 3 siblings, 2 replies; 18+ messages in thread From: Masami Hiramatsu @ 2020-06-12 15:23 UTC (permalink / raw) To: Steven Rostedt; +Cc: stable, LKML Fix /proc/bootconfig to show the correctly choose the double or single quotes according to the value. If a bootconfig value includes a double quote character, we must use single-quotes to quote that value. Fixes: c1a3c36017d4 ("proc: bootconfig: Add /proc/bootconfig to show boot config list") Cc: stable@vger.kernel.org Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- fs/proc/bootconfig.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/fs/proc/bootconfig.c b/fs/proc/bootconfig.c index 9955d75c0585..930d1dae33eb 100644 --- a/fs/proc/bootconfig.c +++ b/fs/proc/bootconfig.c @@ -27,6 +27,7 @@ static int __init copy_xbc_key_value_list(char *dst, size_t size) { struct xbc_node *leaf, *vnode; const char *val; + char q; char *key, *end = dst + size; int ret = 0; @@ -41,16 +42,20 @@ static int __init copy_xbc_key_value_list(char *dst, size_t size) break; dst += ret; vnode = xbc_node_get_child(leaf); - if (vnode && xbc_node_is_array(vnode)) { + if (vnode) { xbc_array_for_each_value(vnode, val) { - ret = snprintf(dst, rest(dst, end), "\"%s\"%s", - val, vnode->next ? ", " : "\n"); + if (strchr(val, '"')) + q = '\''; + else + q = '"'; + ret = snprintf(dst, rest(dst, end), "%c%s%c%s", + q, val, q, vnode->next ? ", " : "\n"); if (ret < 0) goto out; dst += ret; } } else { - ret = snprintf(dst, rest(dst, end), "\"%s\"\n", val); + ret = snprintf(dst, rest(dst, end), "\"\"\n"); if (ret < 0) break; dst += ret; ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value 2020-06-12 15:23 ` [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value Masami Hiramatsu @ 2020-06-12 22:42 ` Masami Hiramatsu 2020-06-15 19:11 ` Steven Rostedt 1 sibling, 0 replies; 18+ messages in thread From: Masami Hiramatsu @ 2020-06-12 22:42 UTC (permalink / raw) To: Masami Hiramatsu; +Cc: Steven Rostedt, stable, LKML, Markus Elfring On Sat, 13 Jun 2020 00:23:18 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > Fix /proc/bootconfig to show the correctly choose the > double or single quotes according to the value. Oops, I missed to remove "show". Fix /proc/bootconfig to correctly choose the double or single quotes according to the value. > > If a bootconfig value includes a double quote character, > we must use single-quotes to quote that value. > > Fixes: c1a3c36017d4 ("proc: bootconfig: Add /proc/bootconfig to show boot config list") > Cc: stable@vger.kernel.org > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > --- > fs/proc/bootconfig.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/fs/proc/bootconfig.c b/fs/proc/bootconfig.c > index 9955d75c0585..930d1dae33eb 100644 > --- a/fs/proc/bootconfig.c > +++ b/fs/proc/bootconfig.c > @@ -27,6 +27,7 @@ static int __init copy_xbc_key_value_list(char *dst, size_t size) > { > struct xbc_node *leaf, *vnode; > const char *val; > + char q; > char *key, *end = dst + size; > int ret = 0; > > @@ -41,16 +42,20 @@ static int __init copy_xbc_key_value_list(char *dst, size_t size) > break; > dst += ret; > vnode = xbc_node_get_child(leaf); > - if (vnode && xbc_node_is_array(vnode)) { > + if (vnode) { > xbc_array_for_each_value(vnode, val) { > - ret = snprintf(dst, rest(dst, end), "\"%s\"%s", > - val, vnode->next ? ", " : "\n"); > + if (strchr(val, '"')) > + q = '\''; > + else > + q = '"'; > + ret = snprintf(dst, rest(dst, end), "%c%s%c%s", > + q, val, q, vnode->next ? ", " : "\n"); > if (ret < 0) > goto out; > dst += ret; > } > } else { > - ret = snprintf(dst, rest(dst, end), "\"%s\"\n", val); > + ret = snprintf(dst, rest(dst, end), "\"\"\n"); > if (ret < 0) > break; > dst += ret; > -- Masami Hiramatsu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value 2020-06-12 15:23 ` [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value Masami Hiramatsu 2020-06-12 22:42 ` Masami Hiramatsu @ 2020-06-15 19:11 ` Steven Rostedt 2020-06-15 19:24 ` Joe Perches 2020-06-16 8:05 ` Masami Hiramatsu 1 sibling, 2 replies; 18+ messages in thread From: Steven Rostedt @ 2020-06-15 19:11 UTC (permalink / raw) To: Masami Hiramatsu; +Cc: stable, LKML On Sat, 13 Jun 2020 00:23:18 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > Fix /proc/bootconfig to show the correctly choose the > double or single quotes according to the value. > > If a bootconfig value includes a double quote character, > we must use single-quotes to quote that value. > > Fixes: c1a3c36017d4 ("proc: bootconfig: Add /proc/bootconfig to show boot config list") > Cc: stable@vger.kernel.org > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > --- > fs/proc/bootconfig.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/fs/proc/bootconfig.c b/fs/proc/bootconfig.c > index 9955d75c0585..930d1dae33eb 100644 > --- a/fs/proc/bootconfig.c > +++ b/fs/proc/bootconfig.c > @@ -27,6 +27,7 @@ static int __init copy_xbc_key_value_list(char *dst, size_t size) > { > struct xbc_node *leaf, *vnode; > const char *val; > + char q; > char *key, *end = dst + size; > int ret = 0; Hmm, shouldn't the above have the upside-down xmas tree format? struct xbc_node *leaf, *vnode; char *key, *end = dst + size; const char *val; char q; int ret = 0; Looks a little better that way. But anyway, more meat below. > > @@ -41,16 +42,20 @@ static int __init copy_xbc_key_value_list(char *dst, size_t size) > break; > dst += ret; > vnode = xbc_node_get_child(leaf); > - if (vnode && xbc_node_is_array(vnode)) { > + if (vnode) { > xbc_array_for_each_value(vnode, val) { > - ret = snprintf(dst, rest(dst, end), "\"%s\"%s", > - val, vnode->next ? ", " : "\n"); The above is a functional change that is not described in the change log. You use to have: if (vnode && xbc_node_is_array(vnode)) { xbc_array_for_each_value() { [..] } } else { [..] } And now have: if (vnode) { xbc_array_for_each_value() { [..] } } else { [..] } Is "vnode" equivalent to "vnode && xbc_node_is_array(vnode)" ? Why was this change made? It seems out of scope with the change log? -- Steve > + if (strchr(val, '"')) > + q = '\''; > + else > + q = '"'; > + ret = snprintf(dst, rest(dst, end), "%c%s%c%s", > + q, val, q, vnode->next ? ", " : "\n"); > if (ret < 0) > goto out; > dst += ret; > } > } else { > - ret = snprintf(dst, rest(dst, end), "\"%s\"\n", val); > + ret = snprintf(dst, rest(dst, end), "\"\"\n"); > if (ret < 0) > break; > dst += ret; ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value 2020-06-15 19:11 ` Steven Rostedt @ 2020-06-15 19:24 ` Joe Perches 2020-06-15 21:21 ` Steven Rostedt 2020-06-16 8:05 ` Masami Hiramatsu 1 sibling, 1 reply; 18+ messages in thread From: Joe Perches @ 2020-06-15 19:24 UTC (permalink / raw) To: Steven Rostedt, Masami Hiramatsu; +Cc: stable, LKML On Mon, 2020-06-15 at 15:11 -0400, Steven Rostedt wrote: > On Sat, 13 Jun 2020 00:23:18 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: [] > > diff --git a/fs/proc/bootconfig.c b/fs/proc/bootconfig.c [] > > @@ -27,6 +27,7 @@ static int __init copy_xbc_key_value_list(char *dst, size_t size) > > { > > struct xbc_node *leaf, *vnode; > > const char *val; > > + char q; > > char *key, *end = dst + size; > > int ret = 0; > > Hmm, shouldn't the above have the upside-down xmas tree format? > > struct xbc_node *leaf, *vnode; > char *key, *end = dst + size; > const char *val; > char q; > int ret = 0; Please don't infect kernel sources with that style oddity. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value 2020-06-15 19:24 ` Joe Perches @ 2020-06-15 21:21 ` Steven Rostedt 2020-06-15 22:30 ` Randy Dunlap 0 siblings, 1 reply; 18+ messages in thread From: Steven Rostedt @ 2020-06-15 21:21 UTC (permalink / raw) To: Joe Perches; +Cc: Masami Hiramatsu, stable, LKML On Mon, 15 Jun 2020 12:24:00 -0700 Joe Perches <joe@perches.com> wrote: > > Hmm, shouldn't the above have the upside-down xmas tree format? > > > > struct xbc_node *leaf, *vnode; > > char *key, *end = dst + size; > > const char *val; > > char q; > > int ret = 0; > > Please don't infect kernel sources with that style oddity. What do you mean? It's already "infected" all over the kernel, (has been for years!) and I kinda like it. It makes reading variables much easier on the eyes, and as I get older, that means a lot more ;-) -- Steve ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value 2020-06-15 21:21 ` Steven Rostedt @ 2020-06-15 22:30 ` Randy Dunlap 2020-06-15 22:42 ` Steven Rostedt 0 siblings, 1 reply; 18+ messages in thread From: Randy Dunlap @ 2020-06-15 22:30 UTC (permalink / raw) To: Steven Rostedt, Joe Perches; +Cc: Masami Hiramatsu, stable, LKML On 6/15/20 2:21 PM, Steven Rostedt wrote: > On Mon, 15 Jun 2020 12:24:00 -0700 > Joe Perches <joe@perches.com> wrote: > >>> Hmm, shouldn't the above have the upside-down xmas tree format? >>> >>> struct xbc_node *leaf, *vnode; >>> char *key, *end = dst + size; >>> const char *val; >>> char q; >>> int ret = 0; >> >> Please don't infect kernel sources with that style oddity. > > What do you mean? It's already "infected" all over the kernel, (has > been for years!) and I kinda like it. It makes reading variables much > easier on the eyes, and as I get older, that means a lot more ;-) Yeah, there is some infection, more in some places than others, but I agree with Joe -- it's not needed or wanted by some of us. -- ~Randy ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value 2020-06-15 22:30 ` Randy Dunlap @ 2020-06-15 22:42 ` Steven Rostedt 2020-06-15 23:12 ` Randy Dunlap 0 siblings, 1 reply; 18+ messages in thread From: Steven Rostedt @ 2020-06-15 22:42 UTC (permalink / raw) To: Randy Dunlap; +Cc: Joe Perches, Masami Hiramatsu, stable, LKML On Mon, 15 Jun 2020 15:30:41 -0700 Randy Dunlap <rdunlap@infradead.org> wrote: > >> Please don't infect kernel sources with that style oddity. > > > > What do you mean? It's already "infected" all over the kernel, (has > > been for years!) and I kinda like it. It makes reading variables much > > easier on the eyes, and as I get older, that means a lot more ;-) > > Yeah, there is some infection, more in some places than others, > but I agree with Joe -- it's not needed or wanted by some of us. We all have preferences. But for code that I need to review, I prefer it. Why would you be bothered by it? Which is easier on the eyes to read variables? struct xbc_node *leaf, *vnode; const char *val; char q; char *key, *end = dst + size; int ret = 0; or struct xbc_node *leaf, *vnode; char *key, *end = dst + size; const char *val; char q; int ret = 0; ? -- Steve ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value 2020-06-15 22:42 ` Steven Rostedt @ 2020-06-15 23:12 ` Randy Dunlap 2020-06-16 5:05 ` Joe Perches 0 siblings, 1 reply; 18+ messages in thread From: Randy Dunlap @ 2020-06-15 23:12 UTC (permalink / raw) To: Steven Rostedt; +Cc: Joe Perches, Masami Hiramatsu, stable, LKML On 6/15/20 3:42 PM, Steven Rostedt wrote: > On Mon, 15 Jun 2020 15:30:41 -0700 > Randy Dunlap <rdunlap@infradead.org> wrote: > >>>> Please don't infect kernel sources with that style oddity. >>> >>> What do you mean? It's already "infected" all over the kernel, (has >>> been for years!) and I kinda like it. It makes reading variables much >>> easier on the eyes, and as I get older, that means a lot more ;-) >> >> Yeah, there is some infection, more in some places than others, >> but I agree with Joe -- it's not needed or wanted by some of us. > > We all have preferences. But for code that I need to review, I prefer > it. > > Why would you be bothered by it? Which is easier on the eyes to read > variables? "to read variables"? I mostly read code, not variables. > struct xbc_node *leaf, *vnode; > const char *val; > char q; > char *key, *end = dst + size; > int ret = 0; > > or > > struct xbc_node *leaf, *vnode; > char *key, *end = dst + size; > const char *val; > char q; > int ret = 0; > > ? But yes, we all have preferences. For data declaration, mine is more like order of use or some grouping having to do with locality. cheers. -- ~Randy Reported-by: Randy Dunlap <rdunlap@infradead.org> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value 2020-06-15 23:12 ` Randy Dunlap @ 2020-06-16 5:05 ` Joe Perches 0 siblings, 0 replies; 18+ messages in thread From: Joe Perches @ 2020-06-16 5:05 UTC (permalink / raw) To: Randy Dunlap, Steven Rostedt; +Cc: Masami Hiramatsu, stable, LKML On Mon, 2020-06-15 at 16:12 -0700, Randy Dunlap wrote: > On 6/15/20 3:42 PM, Steven Rostedt wrote: > > On Mon, 15 Jun 2020 15:30:41 -0700 > > Randy Dunlap <rdunlap@infradead.org> wrote: > > > > > > > Please don't infect kernel sources with that style oddity. > > > > > > > > What do you mean? It's already "infected" all over the kernel, (has > > > > been for years!) Not really. For instance: $ git grep -A6 "^{" fs/proc/*.[ch] > But yes, we all have preferences. For data declaration, mine is more like > order of use or some grouping having to do with locality. > > cheers. Mine too. But a few years ago I submitted this: https://lore.kernel.org/patchwork/patch/732076/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value 2020-06-15 19:11 ` Steven Rostedt 2020-06-15 19:24 ` Joe Perches @ 2020-06-16 8:05 ` Masami Hiramatsu 1 sibling, 0 replies; 18+ messages in thread From: Masami Hiramatsu @ 2020-06-16 8:05 UTC (permalink / raw) To: Steven Rostedt; +Cc: stable, LKML On Mon, 15 Jun 2020 15:11:39 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Sat, 13 Jun 2020 00:23:18 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > Fix /proc/bootconfig to show the correctly choose the > > double or single quotes according to the value. > > > > If a bootconfig value includes a double quote character, > > we must use single-quotes to quote that value. > > > > Fixes: c1a3c36017d4 ("proc: bootconfig: Add /proc/bootconfig to show boot config list") > > Cc: stable@vger.kernel.org > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > > --- > > fs/proc/bootconfig.c | 13 +++++++++---- > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/fs/proc/bootconfig.c b/fs/proc/bootconfig.c > > index 9955d75c0585..930d1dae33eb 100644 > > --- a/fs/proc/bootconfig.c > > +++ b/fs/proc/bootconfig.c > > @@ -27,6 +27,7 @@ static int __init copy_xbc_key_value_list(char *dst, size_t size) > > { > > struct xbc_node *leaf, *vnode; > > const char *val; > > + char q; > > char *key, *end = dst + size; > > int ret = 0; > > Hmm, shouldn't the above have the upside-down xmas tree format? > > struct xbc_node *leaf, *vnode; > char *key, *end = dst + size; > const char *val; > char q; > int ret = 0; > > > Looks a little better that way. But anyway, more meat below. OK. > > > > > @@ -41,16 +42,20 @@ static int __init copy_xbc_key_value_list(char *dst, size_t size) > > break; > > dst += ret; > > vnode = xbc_node_get_child(leaf); > > - if (vnode && xbc_node_is_array(vnode)) { > > + if (vnode) { > > xbc_array_for_each_value(vnode, val) { > > - ret = snprintf(dst, rest(dst, end), "\"%s\"%s", > > - val, vnode->next ? ", " : "\n"); > > The above is a functional change that is not described in the change > log. > > You use to have: > > if (vnode && xbc_node_is_array(vnode)) { > xbc_array_for_each_value() { > [..] > } > } else { > [..] > } > > And now have: > > if (vnode) { > xbc_array_for_each_value() { > [..] > } > } else { > [..] > } > > Is "vnode" equivalent to "vnode && xbc_node_is_array(vnode)" ? No, it's not. But actually, the above change is equivalent, because xbc_array_for_each_value() can handle the vnode has no "next" member. (the array means just "a list of value node") Thus, if (vnode && xbc_node_is_array(vnode)) { xbc_array_for_each_value(vnode) /* vnode->next != NULL */ ... } else { snprintf(val); /* val is an empty string if !vnode */ } is equivalent to if (vnode) { xbc_array_for_each_value(vnode) /* vnode->next can be NULL */ ... } else { snprintf(""); } > > Why was this change made? It seems out of scope with the change log? Because I want to avoid checking double-quote in each value in 2 places. If we don't change the if() code, we need if (strchr(val, '"')) q = '\''; else q = '"'; this in 2 places. Anyway, I'll add it in the patch comment. Thank you, > > -- Steve > > > > + if (strchr(val, '"')) > > + q = '\''; > > + else > > + q = '"'; > > + ret = snprintf(dst, rest(dst, end), "%c%s%c%s", > > + q, val, q, vnode->next ? ", " : "\n"); > > if (ret < 0) > > goto out; > > dst += ret; > > } > > } else { > > - ret = snprintf(dst, rest(dst, end), "\"%s\"\n", val); > > + ret = snprintf(dst, rest(dst, end), "\"\"\n"); > > if (ret < 0) > > break; > > dst += ret; > -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/4] tools/bootconfig: Fix to use correct quotes for value 2020-06-12 15:23 [PATCH 0/4] bootconfig: Fix quote-character issue and return value Masami Hiramatsu 2020-06-12 15:23 ` [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value Masami Hiramatsu @ 2020-06-12 15:23 ` Masami Hiramatsu 2020-06-12 15:23 ` [PATCH 3/4] tools/bootconfig: Fix to return 0 if succeeded to show the bootconfig Masami Hiramatsu 2020-06-12 15:23 ` [PATCH 4/4] tools/bootconfig: Add testcase for show-command and quotes test Masami Hiramatsu 3 siblings, 0 replies; 18+ messages in thread From: Masami Hiramatsu @ 2020-06-12 15:23 UTC (permalink / raw) To: Steven Rostedt; +Cc: stable, LKML Fix bootconfig tool to show the correctly choose the double or single quotes according to the value. If a bootconfig value includes a double quote character, we must use single-quotes to quote that value. Fixes: 950313ebf79c ("tools: bootconfig: Add bootconfig command") Cc: stable@vger.kernel.org Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- tools/bootconfig/main.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c index 0efaf45f7367..21896a6675fd 100644 --- a/tools/bootconfig/main.c +++ b/tools/bootconfig/main.c @@ -14,13 +14,18 @@ #include <linux/kernel.h> #include <linux/bootconfig.h> -static int xbc_show_array(struct xbc_node *node) +static int xbc_show_value(struct xbc_node *node) { const char *val; + char q; int i = 0; xbc_array_for_each_value(node, val) { - printf("\"%s\"%s", val, node->next ? ", " : ";\n"); + if (strchr(val, '"')) + q = '\''; + else + q = '"'; + printf("%c%s%c%s", q, val, q, node->next ? ", " : ";\n"); i++; } return i; @@ -48,10 +53,7 @@ static void xbc_show_compact_tree(void) continue; } else if (cnode && xbc_node_is_value(cnode)) { printf("%s = ", xbc_node_get_data(node)); - if (cnode->next) - xbc_show_array(cnode); - else - printf("\"%s\";\n", xbc_node_get_data(cnode)); + xbc_show_value(cnode); } else { printf("%s;\n", xbc_node_get_data(node)); } ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/4] tools/bootconfig: Fix to return 0 if succeeded to show the bootconfig 2020-06-12 15:23 [PATCH 0/4] bootconfig: Fix quote-character issue and return value Masami Hiramatsu 2020-06-12 15:23 ` [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value Masami Hiramatsu 2020-06-12 15:23 ` [PATCH 2/4] tools/bootconfig: " Masami Hiramatsu @ 2020-06-12 15:23 ` Masami Hiramatsu 2020-06-15 19:14 ` Steven Rostedt 2020-06-12 15:23 ` [PATCH 4/4] tools/bootconfig: Add testcase for show-command and quotes test Masami Hiramatsu 3 siblings, 1 reply; 18+ messages in thread From: Masami Hiramatsu @ 2020-06-12 15:23 UTC (permalink / raw) To: Steven Rostedt; +Cc: stable, LKML 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; + } close(fd); free(buf); ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] tools/bootconfig: Fix to return 0 if succeeded to show the bootconfig 2020-06-12 15:23 ` [PATCH 3/4] tools/bootconfig: Fix to return 0 if succeeded to show the bootconfig Masami Hiramatsu @ 2020-06-15 19:14 ` Steven Rostedt 2020-06-16 7:53 ` Masami Hiramatsu 0 siblings, 1 reply; 18+ messages in thread From: Steven Rostedt @ 2020-06-15 19:14 UTC (permalink / raw) To: Masami Hiramatsu; +Cc: stable, LKML 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] tools/bootconfig: Fix to return 0 if succeeded to show the bootconfig 2020-06-15 19:14 ` Steven Rostedt @ 2020-06-16 7:53 ` Masami Hiramatsu 0 siblings, 0 replies; 18+ messages in thread From: Masami Hiramatsu @ 2020-06-16 7:53 UTC (permalink / raw) To: Steven Rostedt; +Cc: stable, LKML 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> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 4/4] tools/bootconfig: Add testcase for show-command and quotes test 2020-06-12 15:23 [PATCH 0/4] bootconfig: Fix quote-character issue and return value Masami Hiramatsu ` (2 preceding siblings ...) 2020-06-12 15:23 ` [PATCH 3/4] tools/bootconfig: Fix to return 0 if succeeded to show the bootconfig Masami Hiramatsu @ 2020-06-12 15:23 ` Masami Hiramatsu 3 siblings, 0 replies; 18+ messages in thread From: Masami Hiramatsu @ 2020-06-12 15:23 UTC (permalink / raw) To: Steven Rostedt; +Cc: stable, LKML Add testcases for applied bootconfig showing command and double/single quotes issues. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- tools/bootconfig/test-bootconfig.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh index eff16b77d5eb..3c2ab9e75730 100755 --- a/tools/bootconfig/test-bootconfig.sh +++ b/tools/bootconfig/test-bootconfig.sh @@ -55,6 +55,9 @@ echo "Apply command test" xpass $BOOTCONF -a $TEMPCONF $INITRD new_size=$(stat -c %s $INITRD) +echo "Show command test" +xpass $BOOTCONF $INITRD + echo "File size check" xpass test $new_size -eq $(expr $bconf_size + $initrd_size + 9 + 12) @@ -114,6 +117,13 @@ xpass grep -q "bar" $OUTFILE xpass grep -q "baz" $OUTFILE xpass grep -q "qux" $OUTFILE +echo "Double/single quotes test" +echo "key = '\"string\"';" > $TEMPCONF +$BOOTCONF -a $TEMPCONF $INITRD +$BOOTCONF $INITRD > $TEMPCONF +cat $TEMPCONF +xpass grep \'\"string\"\' $TEMPCONF + echo "=== expected failure cases ===" for i in samples/bad-* ; do xfail $BOOTCONF -a $i $INITRD ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value
@ 2020-06-12 16:15 Markus Elfring
2020-06-13 6:55 ` Greg KH
0 siblings, 1 reply; 18+ messages in thread
From: Markus Elfring @ 2020-06-12 16:15 UTC (permalink / raw)
To: Masami Hiramatsu, Steven Rostedt; +Cc: linux-kernel, stable
> Fix /proc/bootconfig to show the correctly choose the
> double or single quotes according to the value.
I suggest to improve this wording a bit.
Regards,
Markus
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value 2020-06-12 16:15 [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value Markus Elfring @ 2020-06-13 6:55 ` Greg KH 0 siblings, 0 replies; 18+ messages in thread From: Greg KH @ 2020-06-13 6:55 UTC (permalink / raw) To: Markus Elfring; +Cc: Masami Hiramatsu, Steven Rostedt, linux-kernel, stable On Fri, Jun 12, 2020 at 06:15:13PM +0200, Markus Elfring wrote: > > Fix /proc/bootconfig to show the correctly choose the > > double or single quotes according to the value. > > I suggest to improve this wording a bit. > > Regards, Hi, This is the semi-friendly patch-bot of Greg Kroah-Hartman. Markus, you seem to have sent a nonsensical or otherwise pointless review comment to a patch submission on a Linux kernel developer mailing list. I strongly suggest that you not do this anymore. Please do not bother developers who are actively working to produce patches and features with comments that, in the end, are a waste of time. Patch submitter, please ignore Markus's suggestion; you do not need to follow it at all. The person/bot/AI that sent it is being ignored by almost all Linux kernel maintainers for having a persistent pattern of behavior of producing distracting and pointless commentary, and inability to adapt to feedback. Please feel free to also ignore emails from them. thanks, greg k-h's patch email bot > Markus ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-06-16 8:05 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-12 15:23 [PATCH 0/4] bootconfig: Fix quote-character issue and return value Masami Hiramatsu 2020-06-12 15:23 ` [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value Masami Hiramatsu 2020-06-12 22:42 ` Masami Hiramatsu 2020-06-15 19:11 ` Steven Rostedt 2020-06-15 19:24 ` Joe Perches 2020-06-15 21:21 ` Steven Rostedt 2020-06-15 22:30 ` Randy Dunlap 2020-06-15 22:42 ` Steven Rostedt 2020-06-15 23:12 ` Randy Dunlap 2020-06-16 5:05 ` Joe Perches 2020-06-16 8:05 ` Masami Hiramatsu 2020-06-12 15:23 ` [PATCH 2/4] tools/bootconfig: " Masami Hiramatsu 2020-06-12 15:23 ` [PATCH 3/4] tools/bootconfig: Fix to return 0 if succeeded to show the bootconfig Masami Hiramatsu 2020-06-15 19:14 ` Steven Rostedt 2020-06-16 7:53 ` Masami Hiramatsu 2020-06-12 15:23 ` [PATCH 4/4] tools/bootconfig: Add testcase for show-command and quotes test Masami Hiramatsu 2020-06-12 16:15 [PATCH 1/4] proc/bootconfig: Fix to use correct quotes for value Markus Elfring 2020-06-13 6:55 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).