linux-kernel - Re: [PATCH v1 1/1] media: drxk: Use %*ph for printing hexdump of a small buffer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b176e4ae-7000-45af-64ba-2a5bd948f1e3@xs4all.nl>
Date:   Wed, 19 Jul 2023 09:53:07 +0200
From:   Hans Verkuil <hverkuil-cisco@...all.nl>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/1] media: drxk: Use %*ph for printing hexdump of a
 small buffer

Hi Andy,

On 12/06/2023 23:20, Andy Shevchenko wrote:
> The kernel already has a helper to print a hexdump of a small
> buffer via pointer extension. Use that instead of open coded
> variant.
> 
> In long term it helps to kill pr_cont() or at least narrow down
> its use.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> ---
>  drivers/media/dvb-frontends/drxk_hard.c | 28 ++++++-------------------
>  1 file changed, 6 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/drxk_hard.c b/drivers/media/dvb-frontends/drxk_hard.c
> index 3301ef75d441..14035deeb6d8 100644
> --- a/drivers/media/dvb-frontends/drxk_hard.c
> +++ b/drivers/media/dvb-frontends/drxk_hard.c
> @@ -229,13 +229,8 @@ static int i2c_write(struct drxk_state *state, u8 adr, u8 *data, int len)
>  	struct i2c_msg msg = {
>  	    .addr = adr, .flags = 0, .buf = data, .len = len };
>  
> -	dprintk(3, ":");
> -	if (debug > 2) {
> -		int i;
> -		for (i = 0; i < len; i++)
> -			pr_cont(" %02x", data[i]);
> -		pr_cont("\n");
> -	}
> +	dprintk(3, ": %*ph\n", len, data);
> +
>  	status = drxk_i2c_transfer(state, &msg, 1);
>  	if (status >= 0 && status != 1)
>  		status = -EIO;
> @@ -268,14 +263,8 @@ static int i2c_read(struct drxk_state *state,
>  		return status;
>  	}
>  	if (debug > 2) {
> -		int i;
> -		dprintk(2, ": read from");
> -		for (i = 0; i < len; i++)
> -			pr_cont(" %02x", msg[i]);
> -		pr_cont(", value = ");
> -		for (i = 0; i < alen; i++)
> -			pr_cont(" %02x", answ[i]);
> -		pr_cont("\n");
> +		dprintk(2, ": read from %*ph, value = %*ph\n", len, msg,
> +			alen, answ);

The patch looks good, except here the dprintk debug level should be 3, not 2.
With that change the 'if (debug > 2)' can be dropped as well.

Regards,

	Hans

>  	}
>  	return 0;
>  }
> @@ -441,13 +430,8 @@ static int write_block(struct drxk_state *state, u32 address,
>  		}
>  		memcpy(&state->chunk[adr_length], p_block, chunk);
>  		dprintk(2, "(0x%08x, 0x%02x)\n", address, flags);
> -		if (debug > 1) {
> -			int i;
> -			if (p_block)
> -				for (i = 0; i < chunk; i++)
> -					pr_cont(" %02x", p_block[i]);
> -			pr_cont("\n");
> -		}
> +		if (p_block)
> +			dprintk(2, "%*ph\n", chunk, p_block);
>  		status = i2c_write(state, state->demod_address,
>  				   &state->chunk[0], chunk + adr_length);
>  		if (status < 0) {