Message ID | 20190410031720.11067-4-alastair@au1.ibm.com (mailing list archive) |
---|---|
State | Not Applicable |
Delegated to: | Johannes Berg |
Headers | show |
Series | Hexdump enhancements | expand |
On Wed, Apr 10, 2019 at 01:17:19PM +1000, Alastair D'Silva wrote: > @@ -107,7 +108,7 @@ EXPORT_SYMBOL(bin2hex); > * string if enough space had been available. > */ > int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > - char *linebuf, size_t linebuflen, bool ascii) > + char *linebuf, size_t linebuflen, u64 flags) > { > const u8 *ptr = buf; > int ngroups; > @@ -184,7 +185,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > if (j) > lx--; > } > - if (!ascii) > + if (!flags & HEXDUMP_ASCII) ^^^^^^^^^^^^^^^^^^^^^^ This is a precedence bug. It should be if (!(flags & HEXDUMP_ASCII)). > goto nil; > regards, dan carpenter
On Wed 2019-04-10 13:17:19, Alastair D'Silva wrote: > From: Alastair D'Silva <alastair@d-silva.org> > > In order to support additional features in hex_dump_to_buffer, replace > the ascii bool parameter with flags. > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org> > --- > drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- > drivers/isdn/hardware/mISDN/mISDNisar.c | 6 ++++-- > drivers/mailbox/mailbox-test.c | 2 +- > drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- > drivers/net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +- > drivers/net/wireless/ath/ath10k/debug.c | 3 ++- > drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +- > drivers/platform/chrome/wilco_ec/debugfs.c | 3 ++- > drivers/scsi/scsi_logging.c | 8 +++----- > drivers/staging/fbtft/fbtft-core.c | 2 +- > fs/seq_file.c | 3 ++- > include/linux/printk.h | 2 +- > lib/hexdump.c | 15 ++++++++------- > lib/test_hexdump.c | 5 +++-- > 14 files changed, 31 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index 49fa43ff02ba..fb133e729f9a 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m, const void *buf, size_t len) > WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos, > rowsize, sizeof(u32), > line, sizeof(line), > - false) >= sizeof(line)); > + 0) >= sizeof(line)); It might be more clear when we define: #define HEXDUMP_BINARY 0 > drm_printf(m, "[%04zx] %s\n", pos, line); > > prev = buf + pos; > diff --git a/include/linux/printk.h b/include/linux/printk.h > index c014e5573665..82975853c400 100644 > --- a/include/linux/printk.h > +++ b/include/linux/printk.h > @@ -493,7 +493,7 @@ enum { > > extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, > int groupsize, char *linebuf, size_t linebuflen, > - bool ascii); > + u64 flags); I wonder how fancy hex_dump could be. IMHO, u32 should be enough. The last famous words ;-) Best Regards, Petr
On 10/04/2019 04:17, Alastair D'Silva wrote: > From: Alastair D'Silva <alastair@d-silva.org> > > In order to support additional features in hex_dump_to_buffer, replace > the ascii bool parameter with flags. > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org> > --- > drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- > drivers/isdn/hardware/mISDN/mISDNisar.c | 6 ++++-- > drivers/mailbox/mailbox-test.c | 2 +- > drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- > drivers/net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +- > drivers/net/wireless/ath/ath10k/debug.c | 3 ++- > drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +- > drivers/platform/chrome/wilco_ec/debugfs.c | 3 ++- > drivers/scsi/scsi_logging.c | 8 +++----- > drivers/staging/fbtft/fbtft-core.c | 2 +- > fs/seq_file.c | 3 ++- > include/linux/printk.h | 2 +- > lib/hexdump.c | 15 ++++++++------- > lib/test_hexdump.c | 5 +++-- > 14 files changed, 31 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index 49fa43ff02ba..fb133e729f9a 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m, const void *buf, size_t len) > WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos, > rowsize, sizeof(u32), > line, sizeof(line), > - false) >= sizeof(line)); > + 0) >= sizeof(line)); > drm_printf(m, "[%04zx] %s\n", pos, line); > > prev = buf + pos; i915 code here actually does something I think is more interesting than HEXDUMP_SUPPRESS_0X**. It replaces any N repeating lines with a single star marker. For example: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 * 00000040 00000001 00000000 00000018 00000002 00000001 00000000 00000018 00000000 00000060 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000003 00000080 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 * 000000c0 00000002 00000000 00000000 00000000 00000000 00000000 00000000 00000000 000000e0 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 * If or when you end up with this feature in your series you can therefore replace the whole implementation of hexdump in the above file. Regards, Tvrtko
> -----Original Message----- > From: Petr Mladek <pmladek@suse.com> > Sent: Saturday, 13 April 2019 12:12 AM > To: Alastair D'Silva <alastair@au1.ibm.com> > Cc: alastair@d-silva.org; Jani Nikula <jani.nikula@linux.intel.com>; Joonas > Lahtinen <joonas.lahtinen@linux.intel.com>; Rodrigo Vivi > <rodrigo.vivi@intel.com>; David Airlie <airlied@linux.ie>; Daniel Vetter > <daniel@ffwll.ch>; Karsten Keil <isdn@linux-pingi.de>; Jassi Brar > <jassisinghbrar@gmail.com>; Tom Lendacky <thomas.lendacky@amd.com>; > David S. Miller <davem@davemloft.net>; Jose Abreu > <Jose.Abreu@synopsys.com>; Kalle Valo <kvalo@codeaurora.org>; > Stanislaw Gruszka <sgruszka@redhat.com>; Benson Leung > <bleung@chromium.org>; Enric Balletbo i Serra > <enric.balletbo@collabora.com>; James E.J. Bottomley > <jejb@linux.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>; > Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Alexander Viro > <viro@zeniv.linux.org.uk>; Sergey Senozhatsky > <sergey.senozhatsky@gmail.com>; Steven Rostedt <rostedt@goodmis.org>; > Andrew Morton <akpm@linux-foundation.org>; intel- > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- > kernel@vger.kernel.org; netdev@vger.kernel.org; > ath10k@lists.infradead.org; linux-wireless@vger.kernel.org; linux- > scsi@vger.kernel.org; linux-fbdev@vger.kernel.org; > devel@driverdev.osuosl.org; linux-fsdevel@vger.kernel.org > Subject: Re: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in > hex_dump_to_buffer with flags > > On Wed 2019-04-10 13:17:19, Alastair D'Silva wrote: > > From: Alastair D'Silva <alastair@d-silva.org> > > > > In order to support additional features in hex_dump_to_buffer, replace > > the ascii bool parameter with flags. > > > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org> > > --- > > drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- > > drivers/isdn/hardware/mISDN/mISDNisar.c | 6 ++++-- > > drivers/mailbox/mailbox-test.c | 2 +- > > drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- > > drivers/net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +- > > drivers/net/wireless/ath/ath10k/debug.c | 3 ++- > > drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +- > > drivers/platform/chrome/wilco_ec/debugfs.c | 3 ++- > > drivers/scsi/scsi_logging.c | 8 +++----- > > drivers/staging/fbtft/fbtft-core.c | 2 +- > > fs/seq_file.c | 3 ++- > > include/linux/printk.h | 2 +- > > lib/hexdump.c | 15 ++++++++------- > > lib/test_hexdump.c | 5 +++-- > > 14 files changed, 31 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > > b/drivers/gpu/drm/i915/intel_engine_cs.c > > index 49fa43ff02ba..fb133e729f9a 100644 > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > > @@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m, const > void *buf, size_t len) > > WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - > pos, > > rowsize, sizeof(u32), > > line, sizeof(line), > > - false) >= sizeof(line)); > > + 0) >= sizeof(line)); > > It might be more clear when we define: > > #define HEXDUMP_BINARY 0 This feels unnecessary, and weird. Omitting the flag won't disable the hex output (as expected), and if you don't want hex output, why call hexdump in the first place? > > drm_printf(m, "[%04zx] %s\n", pos, line); > > > > prev = buf + pos; > > diff --git a/include/linux/printk.h b/include/linux/printk.h index > > c014e5573665..82975853c400 100644 > > --- a/include/linux/printk.h > > +++ b/include/linux/printk.h > > @@ -493,7 +493,7 @@ enum { > > > > extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, > > int groupsize, char *linebuf, size_t linebuflen, > > - bool ascii); > > + u64 flags); > > I wonder how fancy hex_dump could be. IMHO, u32 should be enough. > The last famous words ;-) > > Best Regards, > Petr > > > --- > This email has been checked for viruses by AVG. > https://www.avg.com
On Sat 2019-04-13 09:31:27, Alastair D'Silva wrote: > > -----Original Message----- > > From: Petr Mladek <pmladek@suse.com> > > Sent: Saturday, 13 April 2019 12:12 AM > > To: Alastair D'Silva <alastair@au1.ibm.com> > > Cc: alastair@d-silva.org; Jani Nikula <jani.nikula@linux.intel.com>; > Joonas > > Lahtinen <joonas.lahtinen@linux.intel.com>; Rodrigo Vivi > > <rodrigo.vivi@intel.com>; David Airlie <airlied@linux.ie>; Daniel Vetter > > <daniel@ffwll.ch>; Karsten Keil <isdn@linux-pingi.de>; Jassi Brar > > <jassisinghbrar@gmail.com>; Tom Lendacky <thomas.lendacky@amd.com>; > > David S. Miller <davem@davemloft.net>; Jose Abreu > > <Jose.Abreu@synopsys.com>; Kalle Valo <kvalo@codeaurora.org>; > > Stanislaw Gruszka <sgruszka@redhat.com>; Benson Leung > > <bleung@chromium.org>; Enric Balletbo i Serra > > <enric.balletbo@collabora.com>; James E.J. Bottomley > > <jejb@linux.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>; > > Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Alexander Viro > > <viro@zeniv.linux.org.uk>; Sergey Senozhatsky > > <sergey.senozhatsky@gmail.com>; Steven Rostedt <rostedt@goodmis.org>; > > Andrew Morton <akpm@linux-foundation.org>; intel- > > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- > > kernel@vger.kernel.org; netdev@vger.kernel.org; > > ath10k@lists.infradead.org; linux-wireless@vger.kernel.org; linux- > > scsi@vger.kernel.org; linux-fbdev@vger.kernel.org; > > devel@driverdev.osuosl.org; linux-fsdevel@vger.kernel.org > > Subject: Re: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in > > hex_dump_to_buffer with flags > > > > On Wed 2019-04-10 13:17:19, Alastair D'Silva wrote: > > > From: Alastair D'Silva <alastair@d-silva.org> > > > > > > In order to support additional features in hex_dump_to_buffer, replace > > > the ascii bool parameter with flags. > > > > > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org> > > > --- > > > drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- > > > drivers/isdn/hardware/mISDN/mISDNisar.c | 6 ++++-- > > > drivers/mailbox/mailbox-test.c | 2 +- > > > drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- > > > drivers/net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +- > > > drivers/net/wireless/ath/ath10k/debug.c | 3 ++- > > > drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +- > > > drivers/platform/chrome/wilco_ec/debugfs.c | 3 ++- > > > drivers/scsi/scsi_logging.c | 8 +++----- > > > drivers/staging/fbtft/fbtft-core.c | 2 +- > > > fs/seq_file.c | 3 ++- > > > include/linux/printk.h | 2 +- > > > lib/hexdump.c | 15 ++++++++------- > > > lib/test_hexdump.c | 5 +++-- > > > 14 files changed, 31 insertions(+), 26 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > > > b/drivers/gpu/drm/i915/intel_engine_cs.c > > > index 49fa43ff02ba..fb133e729f9a 100644 > > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > > > @@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m, const > > void *buf, size_t len) > > > WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - > > pos, > > > rowsize, sizeof(u32), > > > line, sizeof(line), > > > - false) >= sizeof(line)); > > > + 0) >= sizeof(line)); > > > > It might be more clear when we define: > > > > #define HEXDUMP_BINARY 0 > > This feels unnecessary, and weird. Omitting the flag won't disable the hex > output (as expected), and if you don't want hex output why call hexdump in > the first place? Why do we have HEXDUMP_ASCII then? Why is the above funtion not using HEXDUMP_ASCII? Who would call it with (HEXDUMP_ASCII | HEXDUMP_BINARY)? Best Regards, Petr
> -----Original Message----- > From: Petr Mladek <pmladek@suse.com> > Sent: Monday, 15 April 2019 7:24 PM > To: Alastair D'Silva <alastair@d-silva.org> > Cc: 'Alastair D'Silva' <alastair@au1.ibm.com>; 'Jani Nikula' > <jani.nikula@linux.intel.com>; 'Joonas Lahtinen' > <joonas.lahtinen@linux.intel.com>; 'Rodrigo Vivi' <rodrigo.vivi@intel.com>; > 'David Airlie' <airlied@linux.ie>; 'Daniel Vetter' <daniel@ffwll.ch>; 'Karsten > Keil' <isdn@linux-pingi.de>; 'Jassi Brar' <jassisinghbrar@gmail.com>; 'Tom > Lendacky' <thomas.lendacky@amd.com>; 'David S. Miller' > <davem@davemloft.net>; 'Jose Abreu' <Jose.Abreu@synopsys.com>; 'Kalle > Valo' <kvalo@codeaurora.org>; 'Stanislaw Gruszka' <sgruszka@redhat.com>; > 'Benson Leung' <bleung@chromium.org>; 'Enric Balletbo i Serra' > <enric.balletbo@collabora.com>; 'James E.J. Bottomley' > <jejb@linux.ibm.com>; 'Martin K. Petersen' <martin.petersen@oracle.com>; > 'Greg Kroah-Hartman' <gregkh@linuxfoundation.org>; 'Alexander Viro' > <viro@zeniv.linux.org.uk>; 'Sergey Senozhatsky' > <sergey.senozhatsky@gmail.com>; 'Steven Rostedt' > <rostedt@goodmis.org>; 'Andrew Morton' <akpm@linux-foundation.org>; > intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- > kernel@vger.kernel.org; netdev@vger.kernel.org; > ath10k@lists.infradead.org; linux-wireless@vger.kernel.org; linux- > scsi@vger.kernel.org; linux-fbdev@vger.kernel.org; > devel@driverdev.osuosl.org; linux-fsdevel@vger.kernel.org > Subject: Re: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in > hex_dump_to_buffer with flags > > On Sat 2019-04-13 09:31:27, Alastair D'Silva wrote: > > > -----Original Message----- > > > From: Petr Mladek <pmladek@suse.com> > > > Sent: Saturday, 13 April 2019 12:12 AM > > > To: Alastair D'Silva <alastair@au1.ibm.com> > > > Cc: alastair@d-silva.org; Jani Nikula <jani.nikula@linux.intel.com>; > > Joonas > > > Lahtinen <joonas.lahtinen@linux.intel.com>; Rodrigo Vivi > > > <rodrigo.vivi@intel.com>; David Airlie <airlied@linux.ie>; Daniel > > > Vetter <daniel@ffwll.ch>; Karsten Keil <isdn@linux-pingi.de>; Jassi > > > Brar <jassisinghbrar@gmail.com>; Tom Lendacky > > > <thomas.lendacky@amd.com>; David S. Miller > <davem@davemloft.net>; > > > Jose Abreu <Jose.Abreu@synopsys.com>; Kalle Valo > > > <kvalo@codeaurora.org>; Stanislaw Gruszka <sgruszka@redhat.com>; > > > Benson Leung <bleung@chromium.org>; Enric Balletbo i Serra > > > <enric.balletbo@collabora.com>; James E.J. Bottomley > > > <jejb@linux.ibm.com>; Martin K. Petersen > > > <martin.petersen@oracle.com>; Greg Kroah-Hartman > > > <gregkh@linuxfoundation.org>; Alexander Viro > > > <viro@zeniv.linux.org.uk>; Sergey Senozhatsky > > > <sergey.senozhatsky@gmail.com>; Steven Rostedt > > > <rostedt@goodmis.org>; Andrew Morton <akpm@linux- > foundation.org>; > > > intel- gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; > > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org; > > > ath10k@lists.infradead.org; linux-wireless@vger.kernel.org; linux- > > > scsi@vger.kernel.org; linux-fbdev@vger.kernel.org; > > > devel@driverdev.osuosl.org; linux-fsdevel@vger.kernel.org > > > Subject: Re: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in > > > hex_dump_to_buffer with flags > > > > > > On Wed 2019-04-10 13:17:19, Alastair D'Silva wrote: > > > > From: Alastair D'Silva <alastair@d-silva.org> > > > > > > > > In order to support additional features in hex_dump_to_buffer, > > > > replace the ascii bool parameter with flags. > > > > > > > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org> > > > > --- > > > > drivers/gpu/drm/i915/intel_engine_cs.c | 2 +- > > > > drivers/isdn/hardware/mISDN/mISDNisar.c | 6 ++++-- > > > > drivers/mailbox/mailbox-test.c | 2 +- > > > > drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- > > > > drivers/net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +- > > > > drivers/net/wireless/ath/ath10k/debug.c | 3 ++- > > > > drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +- > > > > drivers/platform/chrome/wilco_ec/debugfs.c | 3 ++- > > > > drivers/scsi/scsi_logging.c | 8 +++----- > > > > drivers/staging/fbtft/fbtft-core.c | 2 +- > > > > fs/seq_file.c | 3 ++- > > > > include/linux/printk.h | 2 +- > > > > lib/hexdump.c | 15 ++++++++------- > > > > lib/test_hexdump.c | 5 +++-- > > > > 14 files changed, 31 insertions(+), 26 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > > > > b/drivers/gpu/drm/i915/intel_engine_cs.c > > > > index 49fa43ff02ba..fb133e729f9a 100644 > > > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > > > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > > > > @@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m, > > > > const > > > void *buf, size_t len) > > > > WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - > > > pos, > > > > rowsize, sizeof(u32), > > > > line, sizeof(line), > > > > - false) >= sizeof(line)); > > > > + 0) >= sizeof(line)); > > > > > > It might be more clear when we define: > > > > > > #define HEXDUMP_BINARY 0 > > > > This feels unnecessary, and weird. Omitting the flag won't disable the > > hex output (as expected), and if you don't want hex output why call > > hexdump in the first place? > > Why do we have HEXDUMP_ASCII then? > Why is the above funtion not using HEXDUMP_ASCII? > Who would call it with (HEXDUMP_ASCII | HEXDUMP_BINARY)? The ASCII flag controls the optional ASCII output, and replaces the boolean that existed prior. A user would enable it in the same situations where they currently pass true for the ascii parameter. In the above example the author only wants the hex output, while in other situations, both hex & ASCII output is desirable. If you just want ASCII output, the caller should just use a printk or one of it's wrappers.
From: Alastair D'Silva > Sent: 15 April 2019 11:07 ... > In the above example the author only wants the hex output, while in other > situations, both hex & ASCII output is desirable. If you just want ASCII > output, the caller should just use a printk or one of it's wrappers. Hexdump will 'sanitise' the ASCII. Although I think you'd want a 'no hex' flag to suppress the hex. Probably more useful flags are ones to suppress the address column. I've also used flags to enable (or disable) suppression of multiple lines of zeros of constant bytes. In that case you may want hexdump to return the flags for the next call when a large buffer is being dumped in fragments. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
> -----Original Message----- > From: David Laight <David.Laight@ACULAB.COM> > Sent: Monday, 15 April 2019 8:21 PM > To: 'Alastair D'Silva' <alastair@d-silva.org>; 'Petr Mladek' > <pmladek@suse.com> > Cc: 'Alastair D'Silva' <alastair@au1.ibm.com>; 'Jani Nikula' > <jani.nikula@linux.intel.com>; 'Joonas Lahtinen' > <joonas.lahtinen@linux.intel.com>; 'Rodrigo Vivi' <rodrigo.vivi@intel.com>; > 'David Airlie' <airlied@linux.ie>; 'Daniel Vetter' <daniel@ffwll.ch>; 'Karsten > Keil' <isdn@linux-pingi.de>; 'Jassi Brar' <jassisinghbrar@gmail.com>; 'Tom > Lendacky' <thomas.lendacky@amd.com>; 'David S. Miller' > <davem@davemloft.net>; 'Jose Abreu' <Jose.Abreu@synopsys.com>; 'Kalle > Valo' <kvalo@codeaurora.org>; 'Stanislaw Gruszka' <sgruszka@redhat.com>; > 'Benson Leung' <bleung@chromium.org>; 'Enric Balletbo i Serra' > <enric.balletbo@collabora.com>; 'James E.J. Bottomley' > <jejb@linux.ibm.com>; 'Martin K. Petersen' <martin.petersen@oracle.com>; > 'Greg Kroah-Hartman' <gregkh@linuxfoundation.org>; 'Alexander Viro' > <viro@zeniv.linux.org.uk>; 'Sergey Senozhatsky' > <sergey.senozhatsky@gmail.com>; 'Steven Rostedt' > <rostedt@goodmis.org>; 'Andrew Morton' <akpm@linux-foundation.org>; > intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- > kernel@vger.kernel.org; netdev@vger.kernel.org; > ath10k@lists.infradead.org; linux-wireless@vger.kernel.org; linux- > scsi@vger.kernel.org; linux-fbdev@vger.kernel.org; > devel@driverdev.osuosl.org; linux-fsdevel@vger.kernel.org > Subject: RE: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in > hex_dump_to_buffer with flags > > From: Alastair D'Silva > > Sent: 15 April 2019 11:07 > ... > > In the above example the author only wants the hex output, while in > > other situations, both hex & ASCII output is desirable. If you just > > want ASCII output, the caller should just use a printk or one of it's > wrappers. > > Hexdump will 'sanitise' the ASCII. > This is functionality that doesn't exist in the current hexdump implementation (you always get the hex version). I think a better option would be to factor out the sanitisation and expose that as a separate function. > Although I think you'd want a 'no hex' flag to suppress the hex. > > Probably more useful flags are ones to suppress the address column. This is already supported by the prefix_type parameter - are you proposing that we eliminate the parameter & combine it with flags? > I've also used flags to enable (or disable) suppression of multiple lines of > zeros of constant bytes. > In that case you may want hexdump to return the flags for the next call when > a large buffer is being dumped in fragments. I'm afraid I don't quite follow here, hex_dump_to_buffer doesn't alter the flags, so the caller already knows it.
From: Alastair D'Silva > Sent: 15 April 2019 11:45 ... > > Although I think you'd want a 'no hex' flag to suppress the hex. > > > > Probably more useful flags are ones to suppress the address column. > > This is already supported by the prefix_type parameter - are you proposing that we eliminate the > parameter & combine it with flags? I was looking at the flags on one of my hexdump() functions... > > I've also used flags to enable (or disable) suppression of multiple lines of > > zeros of constant bytes. > > In that case you may want hexdump to return the flags for the next call when > > a large buffer is being dumped in fragments. > > I'm afraid I don't quite follow here, hex_dump_to_buffer doesn't alter the flags, > so the caller already knows it. If you are suppressing lines of zeros and dumping a buffer in several blocks then subsequent calls need to know that the last line of the previous call was suppressed zeros - and carry on with the same suppressed block. I've not looked to see if it does support suppressing lines of zeros/0xff. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
> -----Original Message----- > From: David Laight <David.Laight@ACULAB.COM> > Sent: Monday, 15 April 2019 9:04 PM > To: 'Alastair D'Silva' <alastair@d-silva.org>; 'Petr Mladek' > <pmladek@suse.com> > Cc: 'Alastair D'Silva' <alastair@au1.ibm.com>; 'Jani Nikula' > <jani.nikula@linux.intel.com>; 'Joonas Lahtinen' > <joonas.lahtinen@linux.intel.com>; 'Rodrigo Vivi' <rodrigo.vivi@intel.com>; > 'David Airlie' <airlied@linux.ie>; 'Daniel Vetter' <daniel@ffwll.ch>; 'Karsten > Keil' <isdn@linux-pingi.de>; 'Jassi Brar' <jassisinghbrar@gmail.com>; 'Tom > Lendacky' <thomas.lendacky@amd.com>; 'David S. Miller' > <davem@davemloft.net>; 'Jose Abreu' <Jose.Abreu@synopsys.com>; 'Kalle > Valo' <kvalo@codeaurora.org>; 'Stanislaw Gruszka' <sgruszka@redhat.com>; > 'Benson Leung' <bleung@chromium.org>; 'Enric Balletbo i Serra' > <enric.balletbo@collabora.com>; 'James E.J. Bottomley' > <jejb@linux.ibm.com>; 'Martin K. Petersen' <martin.petersen@oracle.com>; > 'Greg Kroah-Hartman' <gregkh@linuxfoundation.org>; 'Alexander Viro' > <viro@zeniv.linux.org.uk>; 'Sergey Senozhatsky' > <sergey.senozhatsky@gmail.com>; 'Steven Rostedt' > <rostedt@goodmis.org>; 'Andrew Morton' <akpm@linux-foundation.org>; > intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- > kernel@vger.kernel.org; netdev@vger.kernel.org; > ath10k@lists.infradead.org; linux-wireless@vger.kernel.org; linux- > scsi@vger.kernel.org; linux-fbdev@vger.kernel.org; > devel@driverdev.osuosl.org; linux-fsdevel@vger.kernel.org > Subject: RE: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in > hex_dump_to_buffer with flags > > From: Alastair D'Silva > > Sent: 15 April 2019 11:45 > ... > > > Although I think you'd want a 'no hex' flag to suppress the hex. > > > > > > Probably more useful flags are ones to suppress the address column. > > > > This is already supported by the prefix_type parameter - are you > > proposing that we eliminate the parameter & combine it with flags? > > I was looking at the flags on one of my hexdump() functions... > > > > I've also used flags to enable (or disable) suppression of multiple > > > lines of zeros of constant bytes. > > > In that case you may want hexdump to return the flags for the next > > > call when a large buffer is being dumped in fragments. > > > > I'm afraid I don't quite follow here, hex_dump_to_buffer doesn't alter > > the flags, so the caller already knows it. > > If you are suppressing lines of zeros and dumping a buffer in several blocks > then subsequent calls need to know that the last line of the previous call was > suppressed zeros - and carry on with the same suppressed block. Why wouldn't you do this with a single call to print_hex_dump? (that is where the repeated lines are suppressed) That will already take chunks of the buffer until the whole thing is output, in what situation do you see a caller chunking the access themselves?
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 49fa43ff02ba..fb133e729f9a 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m, const void *buf, size_t len) WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos, rowsize, sizeof(u32), line, sizeof(line), - false) >= sizeof(line)); + 0) >= sizeof(line)); drm_printf(m, "[%04zx] %s\n", pos, line); prev = buf + pos; diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c b/drivers/isdn/hardware/mISDN/mISDNisar.c index 386731ec2489..f13f34db6c17 100644 --- a/drivers/isdn/hardware/mISDN/mISDNisar.c +++ b/drivers/isdn/hardware/mISDN/mISDNisar.c @@ -84,7 +84,8 @@ send_mbox(struct isar_hw *isar, u8 his, u8 creg, u8 len, u8 *msg) while (l < (int)len) { hex_dump_to_buffer(msg + l, len - l, 32, 1, - isar->log, 256, 1); + isar->log, 256, + HEXDUMP_ASCII); pr_debug("%s: %s %02x: %s\n", isar->name, __func__, l, isar->log); l += 32; @@ -113,7 +114,8 @@ rcv_mbox(struct isar_hw *isar, u8 *msg) while (l < (int)isar->clsb) { hex_dump_to_buffer(msg + l, isar->clsb - l, 32, - 1, isar->log, 256, 1); + 1, isar->log, 256, + HEXDUMP_ASCII); pr_debug("%s: %s %02x: %s\n", isar->name, __func__, l, isar->log); l += 32; diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c index 4e4ac4be6423..2f9a094d0259 100644 --- a/drivers/mailbox/mailbox-test.c +++ b/drivers/mailbox/mailbox-test.c @@ -213,7 +213,7 @@ static ssize_t mbox_test_message_read(struct file *filp, char __user *userbuf, hex_dump_to_buffer(ptr, MBOX_BYTES_PER_LINE, MBOX_BYTES_PER_LINE, 1, touser + l, - MBOX_HEXDUMP_LINE_LEN, true); + MBOX_HEXDUMP_LINE_LEN, HEXDUMP_ASCII); ptr += MBOX_BYTES_PER_LINE; l += MBOX_HEXDUMP_LINE_LEN; diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c index 0cc911f928b1..e954a31cee0c 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c @@ -2992,7 +2992,7 @@ void xgbe_print_pkt(struct net_device *netdev, struct sk_buff *skb, bool tx_rx) unsigned int len = min(skb->len - i, 32U); hex_dump_to_buffer(&skb->data[i], len, 32, 1, - buffer, sizeof(buffer), false); + buffer, sizeof(buffer), 0); netdev_dbg(netdev, " %#06x: %s\n", i, buffer); } diff --git a/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c b/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c index eb1c6b03c329..b80adfa1f890 100644 --- a/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c +++ b/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c @@ -349,7 +349,7 @@ void xlgmac_print_pkt(struct net_device *netdev, unsigned int len = min(skb->len - i, 32U); hex_dump_to_buffer(&skb->data[i], len, 32, 1, - buffer, sizeof(buffer), false); + buffer, sizeof(buffer), 0); netdev_dbg(netdev, " %#06x: %s\n", i, buffer); } diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c index 32d967a31c65..4c99ea03226d 100644 --- a/drivers/net/wireless/ath/ath10k/debug.c +++ b/drivers/net/wireless/ath/ath10k/debug.c @@ -2662,7 +2662,8 @@ void ath10k_dbg_dump(struct ath10k *ar, (unsigned int)(ptr - buf)); hex_dump_to_buffer(ptr, len - (ptr - buf), 16, 1, linebuf + linebuflen, - sizeof(linebuf) - linebuflen, true); + sizeof(linebuf) - linebuflen, + HEXDUMP_ASCII); dev_printk(KERN_DEBUG, ar->dev, "%s\n", linebuf); } } diff --git a/drivers/net/wireless/intel/iwlegacy/3945-mac.c b/drivers/net/wireless/intel/iwlegacy/3945-mac.c index 271977f7fbb0..acbe26d22c34 100644 --- a/drivers/net/wireless/intel/iwlegacy/3945-mac.c +++ b/drivers/net/wireless/intel/iwlegacy/3945-mac.c @@ -3247,7 +3247,7 @@ il3945_show_measurement(struct device *d, struct device_attribute *attr, while (size && PAGE_SIZE - len) { hex_dump_to_buffer(data + ofs, size, 16, 1, buf + len, - PAGE_SIZE - len, true); + PAGE_SIZE - len, HEXDUMP_ASCII); len = strlen(buf); if (PAGE_SIZE - len) buf[len++] = '\n'; diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c index c090db2cd5be..238ede4a26d8 100644 --- a/drivers/platform/chrome/wilco_ec/debugfs.c +++ b/drivers/platform/chrome/wilco_ec/debugfs.c @@ -174,7 +174,8 @@ static ssize_t raw_read(struct file *file, char __user *user_buf, size_t count, fmt_len = hex_dump_to_buffer(debug_info->raw_data, debug_info->response_size, 16, 1, debug_info->formatted_data, - FORMATTED_BUFFER_SIZE, true); + FORMATTED_BUFFER_SIZE, + HEXDUMP_ASCII); /* Only return response the first time it is read */ debug_info->response_size = 0; } diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c index bd70339c1242..fce542bb40e6 100644 --- a/drivers/scsi/scsi_logging.c +++ b/drivers/scsi/scsi_logging.c @@ -263,7 +263,7 @@ void scsi_print_command(struct scsi_cmnd *cmd) "CDB[%02x]: ", k); hex_dump_to_buffer(&cmd->cmnd[k], linelen, 16, 1, logbuf + off, - logbuf_len - off, false); + logbuf_len - off, 0); } dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s", logbuf); @@ -274,8 +274,7 @@ void scsi_print_command(struct scsi_cmnd *cmd) if (!WARN_ON(off > logbuf_len - 49)) { off += scnprintf(logbuf + off, logbuf_len - off, " "); hex_dump_to_buffer(cmd->cmnd, cmd->cmd_len, 16, 1, - logbuf + off, logbuf_len - off, - false); + logbuf + off, logbuf_len - off, 0); } out_printk: dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s", logbuf); @@ -354,8 +353,7 @@ scsi_log_dump_sense(const struct scsi_device *sdev, const char *name, int tag, off = sdev_format_header(logbuf, logbuf_len, name, tag); hex_dump_to_buffer(&sense_buffer[i], len, 16, 1, - logbuf + off, logbuf_len - off, - false); + logbuf + off, logbuf_len - off, 0); dev_printk(KERN_INFO, &sdev->sdev_gendev, "%s", logbuf); } scsi_log_release_buffer(logbuf); diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index 9b07badf4c6c..2e5df5cc9d61 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -61,7 +61,7 @@ void fbtft_dbg_hex(const struct device *dev, int groupsize, va_end(args); hex_dump_to_buffer(buf, len, 32, groupsize, text + text_len, - 512 - text_len, false); + 512 - text_len, 0); if (len > 32) dev_info(dev, "%s ...\n", text); diff --git a/fs/seq_file.c b/fs/seq_file.c index 1dea7a8a5255..a0213637af3e 100644 --- a/fs/seq_file.c +++ b/fs/seq_file.c @@ -873,7 +873,8 @@ void seq_hex_dump(struct seq_file *m, const char *prefix_str, int prefix_type, size = seq_get_buf(m, &buffer); ret = hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize, - buffer, size, ascii); + buffer, size, + ascii ? HEXDUMP_ASCII : 0); seq_commit(m, ret < size ? ret : -1); seq_putc(m, '\n'); diff --git a/include/linux/printk.h b/include/linux/printk.h index c014e5573665..82975853c400 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -493,7 +493,7 @@ enum { extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, char *linebuf, size_t linebuflen, - bool ascii); + u64 flags); #ifdef CONFIG_PRINTK extern void print_hex_dump_ext(const char *level, const char *prefix_str, diff --git a/lib/hexdump.c b/lib/hexdump.c index 2f3bafb55a44..79db784577e7 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -84,7 +84,8 @@ EXPORT_SYMBOL(bin2hex); * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) * @linebuf: where to put the converted data * @linebuflen: total size of @linebuf, including space for terminating NUL - * @ascii: include ASCII after the hex output + * @flags: A bitwise OR of the following flags: + * HEXDUMP_ASCII: include ASCII after the hex output * * hex_dump_to_buffer() works on one "line" of output at a time, i.e., * 16, 32 or 64 bytes of input data converted to hex + ASCII output. @@ -95,7 +96,7 @@ EXPORT_SYMBOL(bin2hex); * * E.g.: * hex_dump_to_buffer(frame->data, frame->len, 16, 1, - * linebuf, sizeof(linebuf), true); + * linebuf, sizeof(linebuf), HEXDUMP_ASCII); * * example output buffer: * 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f @ABCDEFGHIJKLMNO @@ -107,7 +108,7 @@ EXPORT_SYMBOL(bin2hex); * string if enough space had been available. */ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, - char *linebuf, size_t linebuflen, bool ascii) + char *linebuf, size_t linebuflen, u64 flags) { const u8 *ptr = buf; int ngroups; @@ -184,7 +185,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, if (j) lx--; } - if (!ascii) + if (!flags & HEXDUMP_ASCII) goto nil; while (lx < ascii_column) { @@ -204,7 +205,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, overflow2: linebuf[lx++] = '\0'; overflow1: - return ascii ? ascii_column + len : (groupsize * 2 + 1) * ngroups - 1; + return (flags & HEXDUMP_ASCII) ? ascii_column + len : + (groupsize * 2 + 1) * ngroups - 1; } EXPORT_SYMBOL(hex_dump_to_buffer); @@ -299,8 +301,7 @@ void print_hex_dump_ext(const char *level, const char *prefix_str, first_line = false; hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize, - linebuf, sizeof(linebuf), - flags & HEXDUMP_ASCII); + linebuf, sizeof(linebuf), flags); switch (prefix_type) { case DUMP_PREFIX_ADDRESS: diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c index 5144899d3c6b..16bdb483114d 100644 --- a/lib/test_hexdump.c +++ b/lib/test_hexdump.c @@ -132,7 +132,7 @@ static void __init test_hexdump(size_t len, int rowsize, int groupsize, memset(real, FILL_CHAR, sizeof(real)); hex_dump_to_buffer(data_b, len, rowsize, groupsize, real, sizeof(real), - ascii); + ascii ? HEXDUMP_ASCII : 0); memset(test, FILL_CHAR, sizeof(test)); test_hexdump_prepare_test(len, rowsize, groupsize, test, sizeof(test), @@ -171,7 +171,8 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len, memset(buf, FILL_CHAR, sizeof(buf)); - r = hex_dump_to_buffer(data_b, len, rs, gs, buf, buflen, ascii); + r = hex_dump_to_buffer(data_b, len, rs, gs, buf, buflen, + ascii ? HEXDUMP_ASCII : 0); /* * Caller must provide the data length multiple of groupsize. The