From: Petr Mladek <pmladek@suse.com>
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 2/4] lib/hexdump.c: Optionally suppress lines of filler bytes
Date: Mon, 15 Apr 2019 11:18:12 +0200 [thread overview]
Message-ID: <20190415091812.s4e5zlwldbe62ego@pathway.suse.cz> (raw)
In-Reply-To: <093101d4f187$612f0f20$238d2d60$@d-silva.org>
On Sat 2019-04-13 09:28:03, Alastair D'Silva wrote:
> > -----Original Message-----
> > From: Petr Mladek <pmladek@suse.com>
> > Sent: Saturday, 13 April 2019 12:04 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 2/4] lib/hexdump.c: Optionally suppress lines of
> filler
> > bytes
> >
> > On Wed 2019-04-10 13:17:18, Alastair D'Silva wrote:
> > > From: Alastair D'Silva <alastair@d-silva.org>
> > >
> > > Some buffers may only be partially filled with useful data, while the
> > > rest is padded (typically with 0x00 or 0xff).
> > >
> > > This patch introduces flags which allow lines of padding bytes to be
> > > suppressed, making the output easier to interpret:
> > > HEXDUMP_SUPPRESS_0X00, HEXDUMP_SUPPRESS_0XFF
> > >
> > > The first and last lines are not suppressed by default, so the
> > > function always outputs something. This behaviour can be further
> > > controlled with the HEXDUMP_SUPPRESS_FIRST &
> > HEXDUMP_SUPPRESS_LAST flags.
> > >
> > > An inline wrapper function is provided for backwards compatibility
> > > with existing code, which maintains the original behaviour.
> > >
> >
> > > diff --git a/lib/hexdump.c b/lib/hexdump.c index
> > > b8a164814744..2f3bafb55a44 100644
> > > --- a/lib/hexdump.c
> > > +++ b/lib/hexdump.c
> > > +void print_hex_dump_ext(const char *level, const char *prefix_str,
> > > + int prefix_type, int rowsize, int groupsize,
> > > + const void *buf, size_t len, u64 flags)
> > > {
> > > const u8 *ptr = buf;
> > > - int i, linelen, remaining = len;
> > > + int i, remaining = len;
> > > unsigned char linebuf[64 * 3 + 2 + 64 + 1];
> > > + bool first_line = true;
> > >
> > > if (rowsize != 16 && rowsize != 32 && rowsize != 64)
> > > rowsize = 16;
> > >
> > > for (i = 0; i < len; i += rowsize) {
> > > - linelen = min(remaining, rowsize);
> > > + bool skip = false;
> > > + int linelen = min(remaining, rowsize);
> > > +
> > > remaining -= rowsize;
> > >
> > > + if (flags & HEXDUMP_SUPPRESS_0X00)
> > > + skip = buf_is_all(ptr + i, linelen, 0x00);
> > > +
> > > + if (!skip && (flags & HEXDUMP_SUPPRESS_0XFF))
> > > + skip = buf_is_all(ptr + i, linelen, 0xff);
> > > +
> > > + if (first_line && !(flags & HEXDUMP_SUPPRESS_FIRST))
> > > + skip = false;
> > > +
> > > + if (remaining <= 0 && !(flags & HEXDUMP_SUPPRESS_LAST))
> > > + skip = false;
> > > +
> > > + if (skip)
> > > + continue;
> >
> > IMHO, quietly skipping lines could cause a lot of confusion, espcially
> when the address is not printed.
> >
> It's up to the caller to decide how they want it displayed.
I wonder who would want to quietly skip some data values.
Are you using it yourself? Could you please provide an
example?
I do not see why we would need to complicate the API and code
by this.
The behavior proposed by Tvrtko Ursulin makes much more
sense. I mean
https://lkml.kernel.org/r/929244ed-cc7f-b0f3-b5ac-50e798e83188@linux.intel.com
> > I wonder how it would look like when we print something like:
> >
> > --- skipped XX lines full of 0x00 ---
>
> This could be added as a later enhancement, with a new flag (eg.
> HEXDUMP_SUPPRESS_VERBOSE).
Who will add this later? Frankly, this looks like a half baked
feature that it good enough for you. If you want it upstream,
it must behave reasonably and it must be worth it.
Best Regards,
Petr
next prev parent reply other threads:[~2019-04-15 9:18 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-10 3:17 [PATCH 0/4] Hexdump enhancements Alastair D'Silva
2019-04-10 3:17 ` [PATCH 1/4] lib/hexdump.c: Allow 64 bytes per line Alastair D'Silva
2019-04-12 13:48 ` Petr Mladek
2019-04-12 23:22 ` Alastair D'Silva
2019-04-15 9:02 ` Petr Mladek
2019-04-15 10:29 ` Alastair D'Silva
2019-04-15 10:56 ` David Laight
2019-04-15 10:59 ` Alastair D'Silva
2019-04-10 3:17 ` [PATCH 2/4] lib/hexdump.c: Optionally suppress lines of filler bytes Alastair D'Silva
2019-04-10 3:32 ` Alastair D'Silva
2019-04-12 14:03 ` Petr Mladek
2019-04-12 23:28 ` Alastair D'Silva
2019-04-15 9:18 ` Petr Mladek [this message]
2019-04-15 10:33 ` Alastair D'Silva
2019-04-10 3:17 ` [PATCH 3/4] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags Alastair D'Silva
2019-04-10 6:56 ` Dan Carpenter
2019-04-12 14:12 ` Petr Mladek
2019-04-12 23:31 ` Alastair D'Silva
2019-04-15 9:24 ` Petr Mladek
2019-04-15 10:07 ` Alastair D'Silva
2019-04-15 10:20 ` David Laight
2019-04-15 10:44 ` Alastair D'Silva
2019-04-15 11:03 ` David Laight
2019-04-15 11:12 ` Alastair D'Silva
2019-04-12 14:47 ` [Intel-gfx] " Tvrtko Ursulin
2019-04-10 3:17 ` [PATCH 4/4] lib/hexdump.c: Allow multiple groups to be separated by lines '|' Alastair D'Silva
2019-04-10 8:45 ` David Laight
2019-04-10 9:52 ` Alastair D'Silva
2019-04-10 8:53 ` Sergey Senozhatsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190415091812.s4e5zlwldbe62ego@pathway.suse.cz \
--to=pmladek@suse.com \
--cc=Jose.Abreu@synopsys.com \
--cc=airlied@linux.ie \
--cc=akpm@linux-foundation.org \
--cc=alastair@au1.ibm.com \
--cc=alastair@d-silva.org \
--cc=ath10k@lists.infradead.org \
--cc=bleung@chromium.org \
--cc=daniel@ffwll.ch \
--cc=davem@davemloft.net \
--cc=devel@driverdev.osuosl.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=enric.balletbo@collabora.com \
--cc=gregkh@linuxfoundation.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=isdn@linux-pingi.de \
--cc=jani.nikula@linux.intel.com \
--cc=jassisinghbrar@gmail.com \
--cc=jejb@linux.ibm.com \
--cc=joonas.lahtinen@linux.intel.com \
--cc=kvalo@codeaurora.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=netdev@vger.kernel.org \
--cc=rodrigo.vivi@intel.com \
--cc=rostedt@goodmis.org \
--cc=sergey.senozhatsky@gmail.com \
--cc=sgruszka@redhat.com \
--cc=thomas.lendacky@amd.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).