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 = '\'';elseq = '"';ret = snprintf(dst, rest(dst, end), "%c%s%c%s",q, val, q, vnode->next ? ", " : "\n"); if (ret < 0) goto out; dst += ret; }
ret = snprintf(dst, rest(dst, end), "\"%s\"\n", val);
ret = snprintf(dst, rest(dst, end), "\"\"\n"); if (ret < 0) break; dst += ret;