Re: [PATCH v1 01/25] lib/vsprintf: Remove useless NULL checks

From: Rasmus Villemoes
Date: Thu Jun 08 2017 - 16:59:34 EST


On Thu, Jun 08 2017, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:

> The pointer can't be NULL since it's first what has been done in the
> pointer().
>
> Remove useless checks.
>
> Note when we print clock name or rate it is safe in case !CONFIG_HAVE_CLK.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> lib/vsprintf.c | 11 -----------
> 1 file changed, 11 deletions(-)
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 9f16406288c0..031c2cc5c1c0 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -811,10 +811,6 @@ char *hex_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
> /* nothing to print */
> return buf;
>
> - if (ZERO_OR_NULL_PTR(addr))
> - /* NULL pointer */
> - return string(buf, end, NULL, spec);
> -
> switch (fmt[1]) {
> case 'C':
> separator = ':';
> @@ -1253,10 +1249,6 @@ char *escaped_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
> if (spec.field_width == 0)
> return buf; /* nothing to print */
>
> - if (ZERO_OR_NULL_PTR(addr))
> - return string(buf, end, NULL, spec); /* NULL pointer */
> -
> -

Well, ZERO_OR_NULL_PTR checks for a little more than !addr, but I
suppose that if anyone passes the result from kmalloc(0) to %ph, they'd
better also pass 0 as the size, so the .field_width tests should be
sufficient.

> do {
> switch (fmt[count++]) {
> case 'a':
> @@ -1391,9 +1383,6 @@ 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)
> - return string(buf, end, NULL, spec);
> -

Well, it may be safe, but removing the IS_ENABLED(CONFIG_HAVE_CLK) check
means that clock() becomes a much bigger function when
!IS_ENABLED(CONFIG_HAVE_CLK). You're right that the !clk check is
pointless.