Skip to content

Commit

Permalink
vsprintf: Consolidate handling of unknown pointer specifiers
Browse files Browse the repository at this point in the history
There are few printk formats that make sense only with two or more
specifiers. Also some specifiers make sense only when a kernel feature
is enabled.

The handling of unknown specifiers is inconsistent and not helpful.
Using WARN() looks like an overkill for this type of error. pr_warn()
is not good either. It would by handled via printk_safe buffer and
it might be hard to match it with the problematic string.

A reasonable compromise seems to be writing the unknown format specifier
into the original string with a question mark, for example (%pC?).
It should be self-explaining enough. Note that it is in brackets
to follow the (null) style.

Note that it introduces a warning about that test_hashed() function
is unused. It is going to be used again by a later patch.

Link: http://lkml.kernel.org/r/[email protected]
To: Rasmus Villemoes <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: "Tobin C . Harding" <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Sergey Senozhatsky <[email protected]>
Cc: [email protected]
Reviewed-by: Sergey Senozhatsky <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Petr Mladek <[email protected]>
  • Loading branch information
pmladek committed Apr 26, 2019
1 parent 798cc27 commit 0b74d4d
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 13 deletions.
3 changes: 1 addition & 2 deletions lib/test_printf.c
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,7 @@ struct_rtc_time(void)
.tm_year = 118,
};

test_hashed("%pt", &tm);

test("(%ptR?)", "%pt", &tm);
test("2018-11-26T05:35:43", "%ptR", &tm);
test("0118-10-26T05:35:43", "%ptRr", &tm);
test("05:35:43|2018-11-26", "%ptRt|%ptRd", &tm, &tm);
Expand Down
28 changes: 17 additions & 11 deletions lib/vsprintf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1435,6 +1435,8 @@ static noinline_for_stack
char *ip_addr_string(char *buf, char *end, const void *ptr,
struct printf_spec spec, const char *fmt)
{
char *err_fmt_msg;

switch (fmt[1]) {
case '6':
return ip6_addr_string(buf, end, ptr, spec, fmt);
Expand All @@ -1457,7 +1459,8 @@ char *ip_addr_string(char *buf, char *end, const void *ptr,
}}
}

return ptr_to_id(buf, end, ptr, spec);
err_fmt_msg = fmt[0] == 'i' ? "(%pi?)" : "(%pI?)";
return string_nocheck(buf, end, err_fmt_msg, spec);
}

static noinline_for_stack
Expand Down Expand Up @@ -1585,7 +1588,7 @@ char *netdev_bits(char *buf, char *end, const void *addr,
size = sizeof(netdev_features_t);
break;
default:
return ptr_to_id(buf, end, addr, spec);
return string_nocheck(buf, end, "(%pN?)", spec);
}

return special_hex_number(buf, end, num, size);
Expand Down Expand Up @@ -1689,15 +1692,18 @@ char *time_and_date(char *buf, char *end, void *ptr, struct printf_spec spec,
case 'R':
return rtc_str(buf, end, (const struct rtc_time *)ptr, fmt);
default:
return ptr_to_id(buf, end, ptr, spec);
return string_nocheck(buf, end, "(%ptR?)", spec);
}
}

static noinline_for_stack
char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec,
const char *fmt)
{
if (!IS_ENABLED(CONFIG_HAVE_CLK) || !clk)
if (!IS_ENABLED(CONFIG_HAVE_CLK))
return string_nocheck(buf, end, "(%pC?)", spec);

if (!clk)
return string(buf, end, NULL, spec);

switch (fmt[1]) {
Expand All @@ -1706,7 +1712,7 @@ char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec,
#ifdef CONFIG_COMMON_CLK
return string(buf, end, __clk_get_name(clk), spec);
#else
return ptr_to_id(buf, end, clk, spec);
return string_nocheck(buf, end, "(%pC?)", spec);
#endif
}
}
Expand Down Expand Up @@ -1739,7 +1745,8 @@ char *format_flags(char *buf, char *end, unsigned long flags,
}

static noinline_for_stack
char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt)
char *flags_string(char *buf, char *end, void *flags_ptr,
struct printf_spec spec, const char *fmt)
{
unsigned long flags;
const struct trace_print_flags *names;
Expand All @@ -1760,8 +1767,7 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt)
names = gfpflag_names;
break;
default:
WARN_ONCE(1, "Unsupported flags modifier: %c\n", fmt[1]);
return buf;
return string_nocheck(buf, end, "(%pG?)", spec);
}

return format_flags(buf, end, flags, names);
Expand Down Expand Up @@ -1817,7 +1823,7 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
str_spec.field_width = -1;

if (!IS_ENABLED(CONFIG_OF))
return string_nocheck(buf, end, "(!OF)", spec);
return string_nocheck(buf, end, "(%pOF?)", spec);

if ((unsigned long)dn < PAGE_SIZE)
return string_nocheck(buf, end, "(null)", spec);
Expand Down Expand Up @@ -1896,7 +1902,7 @@ static char *kobject_string(char *buf, char *end, void *ptr,
return device_node_string(buf, end, ptr, spec, fmt + 1);
}

return ptr_to_id(buf, end, ptr, spec);
return string_nocheck(buf, end, "(%pO?)", spec);
}

/*
Expand Down Expand Up @@ -2091,7 +2097,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
#endif

case 'G':
return flags_string(buf, end, ptr, fmt);
return flags_string(buf, end, ptr, spec, fmt);
case 'O':
return kobject_string(buf, end, ptr, spec, fmt);
case 'x':
Expand Down

0 comments on commit 0b74d4d

Please sign in to comment.