[lkml]   [2018]   [Apr]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers
On 2018-04-04 10:58, Petr Mladek wrote:
> 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 strange, inconsistent, and
> even leaking the address. For example, netdev_bits() prints the
> non-hashed pointer value or clock() prints "(null)".
> The best solution seems to be in flags_string(). It does not print any
> misleading value. Instead it calls WARN_ONCE() describing the unknown
> specifier. Therefore it clearly shows the problem and helps to find it.

I'm not sure it's actually worth WARNing about the unknown variants
since we have static analysis (at least checkpatch and smatch) that
should catch that. Even just git grep -1 -E '%p"$' finds %pt and %po
which should get fixed before somebody claims those extensions.

But, I don't disagree with trying to fix up the inconsistency, and
certainly not with fixing netdev_bits(), but it seems you've missed that
e.g. the "case: 'g'" is completely compiled out for !CONFIG_BLOCK.
There's also %pOF which is effectively disabled for !CONFIG_OF (which
obviously makes sense), but with yet a different fallback behaviour.

Hm. I think we should somehow distinguish between the cases of "%po" and
"%pNX", i.e. specifiers/variants that are always bogus, and the cases of
a %pOF or %pC that somehow happens even though nobody should have a
struct device_node* or struct clk* to pass.


 \ /
  Last update: 2018-04-05 16:26    [W:1.245 / U:1.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site