linux-kernel - Re: [PATCH] staging: pi433: add rf69_dbg

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: <56b8e6cfc623d53ebba27ee8afbbec0ece58c816.camel@perches.com>
Date:   Fri, 11 Feb 2022 18:59:37 -0800
From:   Joe Perches <joe@...ches.com>
To:     Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>
Cc:     realwakka@...il.com, linux-staging@...ts.linux.dev,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: pi433: add rf69_dbg_hex function

On Sat, 2022-02-12 at 08:39 +1300, Paulo Miguel Almeida wrote:
> On Fri, Feb 11, 2022 at 09:14:40AM +0100, Greg KH wrote:
> > 
> > This is a lot of additional complexity for almost no real benefit.
> > 
> 
> you're right. I will no longer pursue this approach. 
> 
> > > -	/* print content read from fifo for debugging purposes */
> > > -	for (i = 0; i < size; i++)
> > > -		dev_dbg(&spi->dev, "%d - 0x%x\n", i, local_buffer[i + 1]);
> > 
> > What is wrong with this simple line?
> > 
> 
> to be honest, I think that 1 register per line isn't the easiest way to
> read them. Given that print_hex_dump_debug existed and had this
> horizontal-style priting format, I thought that it would be a better
> way of visualizing the fifo data.
> 
> the only problems with print_hex_dump_debug was the absense of device
> name and string format... so I saw a couple of drivers implementing
> alternative hex_dump-like functions and thought that pi433 would benefit
> from similar approach.
> 
> > > -	/* print content written from fifo for debugging purposes */
> > > -	for (i = 0; i < size; i++)
> > > -		dev_dbg(&spi->dev, "0x%x\n", buffer[i]);
> 
> if we are keeping this format, I may need to add the register idx to
> dev_dbg: 
> 		dev_dbg(&spi->dev, "%d - 0x%x\n", i, buffer[i]);

You could use %*ph with a buffer length up to 64 bytes.

Multiple times if necessary. 

Something like:

	for (i = 0; i < size; i += 64)
		dev_dbg(&spi->dev, "FIFO buffer: %d: %*ph\n",
			i, min_t(int, 64, size - i), buffer + i);

Or for a more realistic length like 16

	for (i = 0; i < size; i += 16)
		dev_dbg(&spi->dev, "FIFO buffer: %d: %*ph\n",
			i, min_t(int, 16, size - i), buffer + i);