linux-kernel - Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1449691768.25389.51.camel@perches.com>
Date:	Wed, 09 Dec 2015 12:09:28 -0800
From:	Joe Perches <joe@...ches.com>
To:	Andy Shevchenko <andy.shevchenko@...il.com>
Cc:	Rob Herring <robh+dt@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Masahiro Yamada <yamada.masahiro@...ionext.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Frank Rowand <frowand.list@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Grant Likely <grant.likely@...aro.org>,
	Rasmus Villemoes <linux@...musvillemoes.dk>
Subject: Re: [PATCH v2] of/address: replace printk() with pr_debug() /
 pr_err()

On Wed, 2015-12-09 at 22:02 +0200, Andy Shevchenko wrote:
> On Wed, Dec 9, 2015 at 9:28 PM, Joe Perches <joe@...ches.com> wrote:
[]
> > > > diff --git a/lib/hexdump.c b/lib/hexdump.c
> > > > index 992457b..49113aa 100644
> > > > --- a/lib/hexdump.c
> > > > +++ b/lib/hexdump.c
> > > > @@ -81,6 +81,7 @@ EXPORT_SYMBOL(bin2hex);
> > > >   * @len: number of bytes in the @buf
> > > >   * @rowsize: number of bytes to print per line; must be 16 or 32
> > > >   * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
> > > > + *             OR'd with DUMP_TYPE_BE or DUMP_TYPE_LE for endian conversions
> > > >   * @linebuf: where to put the converted data
> > > >   * @linebuflen: total size of @linebuf, including space for terminating NUL
> > > >   * @ascii: include ASCII after the hex output
> > > > @@ -114,19 +115,20 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
> > > >         int j, lx = 0;
> > > >         int ascii_column;
> > > >         int ret;
> > > > +       int actual_groupsize = groupsize & ~(DUMP_TYPE_LE | DUMP_TYPE_BE);
> > > 
> > > I would rather prefer to have function parameter to be renamed.
> > > 
> > > E.g. gsflags ?
> > > 
> > 
> > Well, it's a bit simpler changelog.
> 
> Generally I'm fine with this version, though take into account above and below.
> 
> > ---
> >  include/linux/printk.h |  7 +++++++
> >  lib/hexdump.c          | 39 +++++++++++++++++++++++++++++++--------
> >  2 files changed, 38 insertions(+), 8 deletions(-)
> > 
> > diff --git a/include/linux/printk.h b/include/linux/printk.h
> > index 9729565..4be190c 100644
> > --- a/include/linux/printk.h
> > +++ b/include/linux/printk.h
> > @@ -424,6 +424,13 @@ enum {
> >         DUMP_PREFIX_ADDRESS,
> >         DUMP_PREFIX_OFFSET
> >  };
> > +
> > +enum {
> > +       DUMP_TYPE_CPU = 0,
> 
> And still open this, do we need it? I think you may just mention in
> the documentation that default behaviour is CPU like.

The only documentation I'm aware of is the kernel-doc

> > +       DUMP_TYPE_LE = BIT(30),
> > +       DUMP_TYPE_BE = BIT(31)
> > +};
> > +
> >  extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
> >                               int groupsize, char *linebuf, size_t linebuflen,
> 
> Here as well to change.

Right, thanks.

> >                               bool ascii);
> > diff --git a/lib/hexdump.c b/lib/hexdump.c
> > index 992457b..5b1eda70 100644
> > --- a/lib/hexdump.c
> > +++ b/lib/hexdump.c
> > @@ -80,7 +80,8 @@ EXPORT_SYMBOL(bin2hex);
> >   * @buf: data blob to dump
> >   * @len: number of bytes in the @buf
> >   * @rowsize: number of bytes to print per line; must be 16 or 32
> > - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
> > + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1)
> > + *              OR with DUMP_TYPE_BE or DUMP_TYPE_LE for endian conversions
> 
> Maybe specify "bitwise OR with.." ?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/