Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1286632 > unrolled thread
| Started by | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| First post | 2015-12-08 17:10 +0100 |
| Last post | 2015-12-10 07:30 +0100 |
| Articles | 20 on this page of 23 — 7 participants |
Back to article view | Back to linux.kernel
[PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-08 17:10 +0100
Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Joe Perches <joe@perches.com> - 2015-12-08 17:20 +0100
Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Joe Perches <joe@perches.com> - 2015-12-08 18:10 +0100
Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Rob Herring <robh+dt@kernel.org> - 2015-12-08 22:30 +0100
Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Joe Perches <joe@perches.com> - 2015-12-09 01:10 +0100
Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-12-09 13:10 +0100
Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Joe Perches <joe@perches.com> - 2015-12-09 20:30 +0100
Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-12-09 21:10 +0100
Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Joe Perches <joe@perches.com> - 2015-12-09 21:20 +0100
Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-09 21:20 +0100
Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-12-09 21:40 +0100
Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Joe Perches <joe@perches.com> - 2015-12-09 21:50 +0100
[PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Joe Perches <joe@perches.com> - 2015-12-09 23:00 +0100
Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Andrew Morton <akpm@linux-foundation.org> - 2015-12-09 23:10 +0100
Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Joe Perches <joe@perches.com> - 2015-12-09 23:20 +0100
Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Andrew Morton <akpm@linux-foundation.org> - 2015-12-09 23:50 +0100
Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Joe Perches <joe@perches.com> - 2015-12-10 00:10 +0100
Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-12-09 23:20 +0100
Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Joe Perches <joe@perches.com> - 2015-12-09 23:30 +0100
Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-12-10 00:10 +0100
Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Joe Perches <joe@perches.com> - 2015-12-10 00:20 +0100
Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions kbuild test robot <lkp@intel.com> - 2015-12-10 07:00 +0100
Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Joe Perches <joe@perches.com> - 2015-12-10 07:30 +0100
Page 1 of 2 [1] 2 Next page →
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2015-12-08 17:10 +0100 |
| Subject | [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() |
| Message-ID | <qDsZZ-4yl-37@gated-at.bofh.it> |
Trivial changes suggested by checkpatch.pl.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v2:
- Fix printk(KERN_DEBUG ...) too
drivers/of/address.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 9582c57..65fafbb 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev,
#ifdef DEBUG
static void of_dump_addr(const char *s, const __be32 *addr, int na)
{
- printk(KERN_DEBUG "%s", s);
+ pr_debug("%s", s);
while (na--)
printk(" %08x", be32_to_cpu(*(addr++)));
printk("\n");
@@ -597,7 +597,7 @@ static u64 __of_translate_address(struct device_node *dev,
pbus = of_match_bus(parent);
pbus->count_cells(dev, &pna, &pns);
if (!OF_CHECK_COUNTS(pna, pns)) {
- printk(KERN_ERR "prom_parse: Bad cell count for %s\n",
+ pr_err("prom_parse: Bad cell count for %s\n",
of_node_full_name(dev));
break;
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-12-08 17:20 +0100 |
| Subject | Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() |
| Message-ID | <qDt9E-4CC-27@gated-at.bofh.it> |
| In reply to | #1286632 |
On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote:
> Trivial changes suggested by checkpatch.pl.
[]
> diff --git a/drivers/of/address.c b/drivers/of/address.c
[]
> @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev,
> #ifdef DEBUG
> static void of_dump_addr(const char *s, const __be32 *addr, int na)
> {
> - printk(KERN_DEBUG "%s", s);
> + pr_debug("%s", s);
> while (na--)
> printk(" %08x", be32_to_cpu(*(addr++)));
> printk("\n");
mixing pr_debug and printk doesn't make much sense.
It might be nicer to use
static void of_dumpaddr(const char *s, const __be32 *addr, int na)
{
#ifdef DEBUG
...
#endif
}
to avoid the other static declaration
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-12-08 18:10 +0100 |
| Subject | Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() |
| Message-ID | <qDtW2-59U-25@gated-at.bofh.it> |
| In reply to | #1286646 |
On Tue, 2015-12-08 at 08:16 -0800, Joe Perches wrote:
> On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote:
> > Trivial changes suggested by checkpatch.pl.
> []
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> []
> > @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev,
> > #ifdef DEBUG
> > static void of_dump_addr(const char *s, const __be32 *addr, int na)
> > {
> > - printk(KERN_DEBUG "%s", s);
> > + pr_debug("%s", s);
> > while (na--)
> > printk(" %08x", be32_to_cpu(*(addr++)));
> > printk("\n");
>
> mixing pr_debug and printk doesn't make much sense.
>
> It might be nicer to use
>
> static void of_dumpaddr(const char *s, const __be32 *addr, int na)
> {
> #ifdef DEBUG
> ...
> #endif
> }
>
> to avoid the other static declaration
Or more simply:
static void of_dumpaddr(const char *s, const __be32 *addr, int na)
{
print_hex_dump_debug(s, DUMP_PREFIX_NONE, 16, 1,
addr, na * sizeof(__be32), false);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh+dt@kernel.org> |
|---|---|
| Date | 2015-12-08 22:30 +0100 |
| Message-ID | <qDxZD-7Cj-1@gated-at.bofh.it> |
| In reply to | #1286700 |
On Tue, Dec 8, 2015 at 11:03 AM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2015-12-08 at 08:16 -0800, Joe Perches wrote:
>> On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote:
>> > Trivial changes suggested by checkpatch.pl.
>> []
>> > diff --git a/drivers/of/address.c b/drivers/of/address.c
>> []
>> > @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev,
>> > #ifdef DEBUG
>> > static void of_dump_addr(const char *s, const __be32 *addr, int na)
>> > {
>> > - printk(KERN_DEBUG "%s", s);
>> > + pr_debug("%s", s);
>> > while (na--)
>> > printk(" %08x", be32_to_cpu(*(addr++)));
>> > printk("\n");
>>
>> mixing pr_debug and printk doesn't make much sense.
>>
>> It might be nicer to use
>>
>> static void of_dumpaddr(const char *s, const __be32 *addr, int na)
>> {
>> #ifdef DEBUG
>> ...
>> #endif
>> }
>>
>> to avoid the other static declaration
>
> Or more simply:
>
> static void of_dumpaddr(const char *s, const __be32 *addr, int na)
> {
> print_hex_dump_debug(s, DUMP_PREFIX_NONE, 16, 1,
> addr, na * sizeof(__be32), false);
> }
Except that it doesn't do the endian swapping. Looking closer at this,
we should just drop this hunk. So I will take v1.
Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-12-09 01:10 +0100 |
| Subject | Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() |
| Message-ID | <qDAuu-Rv-1@gated-at.bofh.it> |
| In reply to | #1286856 |
On Tue, 2015-12-08 at 15:28 -0600, Rob Herring wrote:
> On Tue, Dec 8, 2015 at 11:03 AM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2015-12-08 at 08:16 -0800, Joe Perches wrote:
> > > On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote:
> > > > Trivial changes suggested by checkpatch.pl.
> > > []
> > > > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > > []
> > > > @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev,
> > > > #ifdef DEBUG
> > > > static void of_dump_addr(const char *s, const __be32 *addr, int na)
> > > > {
> > > > - printk(KERN_DEBUG "%s", s);
> > > > + pr_debug("%s", s);
> > > > while (na--)
> > > > printk(" %08x", be32_to_cpu(*(addr++)));
> > > > printk("\n");
[]
> > static void of_dumpaddr(const char *s, const __be32 *addr, int na)
> > {
> > print_hex_dump_debug(s, DUMP_PREFIX_NONE, 16, 1,
> > addr, na * sizeof(__be32), false);
> > }
>
> Except that it doesn't do the endian swapping. Looking closer at this,
> we should just drop this hunk. So I will take v1.
Maybe endian conversions should be added to hex_dump_debug like:
(probably doesn't apply. Evolution 3.18 is horrible)
---
include/linux/printk.h | 7 +++++++
lib/hexdump.c | 56 +++++++++++++++++++++++++++++++++++---------------
2 files changed, 47 insertions(+), 16 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,
+ 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,
bool ascii);
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);
if (rowsize != 16 && rowsize != 32)
rowsize = 16;
if (len > rowsize) /* limit to one line at a time */
len = rowsize;
- if (!is_power_of_2(groupsize) || groupsize > 8)
- groupsize = 1;
- if ((len % groupsize) != 0) /* no mixed size output */
- groupsize = 1;
+ if (!is_power_of_2(actual_groupsize) || actual_groupsize > 8)
+ actual_groupsize = 1;
+ if ((len % actual_groupsize) != 0) /* no mixed size output */
+ actual_groupsize = 1;
- ngroups = len / groupsize;
- ascii_column = rowsize * 2 + rowsize / groupsize + 1;
+ ngroups = len / actual_groupsize;
+ ascii_column = rowsize * 2 + rowsize / actual_groupsize + 1;
if (!linebuflen)
goto overflow1;
@@ -134,35 +136,56 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
if (!len)
goto nil;
- if (groupsize == 8) {
+ if (actual_groupsize == 8) {
const u64 *ptr8 = buf;
for (j = 0; j < ngroups; j++) {
+ u64 val;
+
+ if (groupsize & DUMP_TYPE_LE)
+ val = get_unaligned_le64(ptr8 + j);
+ else if (groupsize & DUMP_TYPE_BE)
+ val = get_unaligned_be64(ptr8 + j);
+ else
+ val = get_unaligned(ptr8 + j);
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%16.16llx", j ? " " : "",
- get_unaligned(ptr8 + j));
+ "%s%16.16llx", j ? " " : "", val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
}
- } else if (groupsize == 4) {
+ } else if (actual_groupsize == 4) {
const u32 *ptr4 = buf;
for (j = 0; j < ngroups; j++) {
+ u32 val;
+
+ if (groupsize & DUMP_TYPE_LE)
+ val = get_unaligned_le32(ptr4 + j);
+ else if (groupsize & DUMP_TYPE_BE)
+ val = get_unaligned_be32(ptr4 + j);
+ else
+ val = get_unaligned(ptr4 + j);
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%8.8x", j ? " " : "",
- get_unaligned(ptr4 + j));
+ "%s%8.8x", j ? " " : "", val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
}
- } else if (groupsize == 2) {
+ } else if (actual_groupsize == 2) {
const u16 *ptr2 = buf;
for (j = 0; j < ngroups; j++) {
+ u16 val;
+
+ if (groupsize & DUMP_TYPE_LE)
+ val = get_unaligned_le16(ptr2 + j);
+ else if (groupsize & DUMP_TYPE_BE)
+ val = get_unaligned_be16(ptr2 + j);
+ else
+ val = get_unaligned(ptr2 + j);
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%4.4x", j ? " " : "",
- get_unaligned(ptr2 + j));
+ "%s%4.4x", j ? " " : "", val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -203,7 +226,8 @@ nil:
overflow2:
linebuf[lx++] = '\0';
overflow1:
- return ascii ? ascii_column + len : (groupsize * 2 + 1) * ngroups - 1;
+ return ascii ? ascii_column + len
+ : (actual_groupsize * 2 + 1) * ngroups - 1;
}
EXPORT_SYMBOL(hex_dump_to_buffer);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Andy Shevchenko <andy.shevchenko@gmail.com> |
|---|---|
| Date | 2015-12-09 13:10 +0100 |
| Message-ID | <qDLJg-8df-1@gated-at.bofh.it> |
| In reply to | #1286925 |
On Wed, Dec 9, 2015 at 2:03 AM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2015-12-08 at 15:28 -0600, Rob Herring wrote:
>> On Tue, Dec 8, 2015 at 11:03 AM, Joe Perches <joe@perches.com> wrote:
>> > On Tue, 2015-12-08 at 08:16 -0800, Joe Perches wrote:
>> > > On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote:
>> > > > Trivial changes suggested by checkpatch.pl.
>> > > []
>> > > > diff --git a/drivers/of/address.c b/drivers/of/address.c
>> > > []
>> > > > @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev,
>> > > > #ifdef DEBUG
>> > > > static void of_dump_addr(const char *s, const __be32 *addr, int na)
>> > > > {
>> > > > - printk(KERN_DEBUG "%s", s);
>> > > > + pr_debug("%s", s);
>> > > > while (na--)
>> > > > printk(" %08x", be32_to_cpu(*(addr++)));
>> > > > printk("\n");
> []
>> > static void of_dumpaddr(const char *s, const __be32 *addr, int na)
>> > {
>> > print_hex_dump_debug(s, DUMP_PREFIX_NONE, 16, 1,
>> > addr, na * sizeof(__be32), false);
>> > }
>>
>> Except that it doesn't do the endian swapping. Looking closer at this,
>> we should just drop this hunk. So I will take v1.
>
> Maybe endian conversions should be added to hex_dump_debug like:
> (probably doesn't apply. Evolution 3.18 is horrible)
Overall the idea is not bad I think, though few comments below.
Besides that, please, update test_hexdump (wrt my update to the test
suite [1]) to go through BE test cases.
[1] http://www.spinics.net/lists/kernel/msg2139337.html
> ---
> include/linux/printk.h | 7 +++++++
> lib/hexdump.c | 56 +++++++++++++++++++++++++++++++++++---------------
> 2 files changed, 47 insertions(+), 16 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,
Do we need explicit enum for that? Documentation update anyway is required.
> + 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,
> bool ascii);
> 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 ?
>
> if (rowsize != 16 && rowsize != 32)
> rowsize = 16;
>
> if (len > rowsize) /* limit to one line at a time */
> len = rowsize;
> - if (!is_power_of_2(groupsize) || groupsize > 8)
> - groupsize = 1;
> - if ((len % groupsize) != 0) /* no mixed size output */
> - groupsize = 1;
> + if (!is_power_of_2(actual_groupsize) || actual_groupsize > 8)
> + actual_groupsize = 1;
> + if ((len % actual_groupsize) != 0) /* no mixed size output */
> + actual_groupsize = 1;
>
> - ngroups = len / groupsize;
> - ascii_column = rowsize * 2 + rowsize / groupsize + 1;
> + ngroups = len / actual_groupsize;
> + ascii_column = rowsize * 2 + rowsize / actual_groupsize + 1;
>
> if (!linebuflen)
> goto overflow1;
> @@ -134,35 +136,56 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
> if (!len)
> goto nil;
>
> - if (groupsize == 8) {
> + if (actual_groupsize == 8) {
> const u64 *ptr8 = buf;
>
> for (j = 0; j < ngroups; j++) {
> + u64 val;
> +
> + if (groupsize & DUMP_TYPE_LE)
> + val = get_unaligned_le64(ptr8 + j);
> + else if (groupsize & DUMP_TYPE_BE)
> + val = get_unaligned_be64(ptr8 + j);
> + else
> + val = get_unaligned(ptr8 + j);
> ret = snprintf(linebuf + lx, linebuflen - lx,
> - "%s%16.16llx", j ? " " : "",
> - get_unaligned(ptr8 + j));
> + "%s%16.16llx", j ? " " : "", val);
> if (ret >= linebuflen - lx)
> goto overflow1;
> lx += ret;
> }
> - } else if (groupsize == 4) {
> + } else if (actual_groupsize == 4) {
> const u32 *ptr4 = buf;
>
> for (j = 0; j < ngroups; j++) {
> + u32 val;
> +
> + if (groupsize & DUMP_TYPE_LE)
> + val = get_unaligned_le32(ptr4 + j);
> + else if (groupsize & DUMP_TYPE_BE)
> + val = get_unaligned_be32(ptr4 + j);
> + else
> + val = get_unaligned(ptr4 + j);
> ret = snprintf(linebuf + lx, linebuflen - lx,
> - "%s%8.8x", j ? " " : "",
> - get_unaligned(ptr4 + j));
> + "%s%8.8x", j ? " " : "", val);
> if (ret >= linebuflen - lx)
> goto overflow1;
> lx += ret;
> }
> - } else if (groupsize == 2) {
> + } else if (actual_groupsize == 2) {
> const u16 *ptr2 = buf;
>
> for (j = 0; j < ngroups; j++) {
> + u16 val;
> +
> + if (groupsize & DUMP_TYPE_LE)
> + val = get_unaligned_le16(ptr2 + j);
> + else if (groupsize & DUMP_TYPE_BE)
> + val = get_unaligned_be16(ptr2 + j);
> + else
> + val = get_unaligned(ptr2 + j);
> ret = snprintf(linebuf + lx, linebuflen - lx,
> - "%s%4.4x", j ? " " : "",
> - get_unaligned(ptr2 + j));
> + "%s%4.4x", j ? " " : "", val);
> if (ret >= linebuflen - lx)
> goto overflow1;
> lx += ret;
> @@ -203,7 +226,8 @@ nil:
> overflow2:
> linebuf[lx++] = '\0';
> overflow1:
> - return ascii ? ascii_column + len : (groupsize * 2 + 1) * ngroups - 1;
> + return ascii ? ascii_column + len
> + : (actual_groupsize * 2 + 1) * ngroups - 1;
> }
> EXPORT_SYMBOL(hex_dump_to_buffer);
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-12-09 20:30 +0100 |
| Subject | Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() |
| Message-ID | <qDSB5-460-49@gated-at.bofh.it> |
| In reply to | #1287445 |
On Wed, 2015-12-09 at 14:03 +0200, Andy Shevchenko wrote:
> On Wed, Dec 9, 2015 at 2:03 AM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2015-12-08 at 15:28 -0600, Rob Herring wrote:
> > > On Tue, Dec 8, 2015 at 11:03 AM, Joe Perches <joe@perches.com> wrote:
> > > > On Tue, 2015-12-08 at 08:16 -0800, Joe Perches wrote:
> > > > > On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote:
> > > > > > Trivial changes suggested by checkpatch.pl.
> > > > > []
> > > > > > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > > > > []
> > > > > > @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev,
> > > > > > #ifdef DEBUG
> > > > > > static void of_dump_addr(const char *s, const __be32 *addr, int na)
> > > > > > {
> > > > > > - printk(KERN_DEBUG "%s", s);
> > > > > > + pr_debug("%s", s);
> > > > > > while (na--)
> > > > > > printk(" %08x", be32_to_cpu(*(addr++)));
> > > > > > printk("\n");
> > []
> > > > static void of_dumpaddr(const char *s, const __be32 *addr, int na)
> > > > {
> > > > print_hex_dump_debug(s, DUMP_PREFIX_NONE, 16, 1,
> > > > addr, na * sizeof(__be32), false);
> > > > }
> > >
> > > Except that it doesn't do the endian swapping. Looking closer at this,
> > > we should just drop this hunk. So I will take v1.
> >
> > Maybe endian conversions should be added to hex_dump_debug like:
> > (probably doesn't apply. Evolution 3.18 is horrible)
>
> Overall the idea is not bad I think, though few comments below.
> Besides that, please, update test_hexdump (wrt my update to the test
> suite [1]) to go through BE test cases.
>
> [1] http://www.spinics.net/lists/kernel/msg2139337.html
I don't want to have to learn that code as it seems a bit tricky.
Maybe you could do it.
> > 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.
---
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,
+ 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,
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
* @linebuf: where to put the converted data
* @linebuflen: total size of @linebuf, including space for terminating NUL
* @ascii: include ASCII after the hex output
@@ -105,7 +106,7 @@ EXPORT_SYMBOL(bin2hex);
* (excluding the terminating NUL) which would have been written to the final
* string if enough space had been available.
*/
-int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
+int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupflags,
char *linebuf, size_t linebuflen, bool ascii)
{
const u8 *ptr = buf;
@@ -114,6 +115,7 @@ 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 groupsize = groupflags & ~(DUMP_TYPE_LE | DUMP_TYPE_BE);
if (rowsize != 16 && rowsize != 32)
rowsize = 16;
@@ -138,9 +140,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
const u64 *ptr8 = buf;
for (j = 0; j < ngroups; j++) {
+ u64 val;
+
+ if (groupflags & DUMP_TYPE_LE)
+ val = get_unaligned_le64(ptr8 + j);
+ else if (groupflags & DUMP_TYPE_BE)
+ val = get_unaligned_be64(ptr8 + j);
+ else
+ val = get_unaligned(ptr8 + j);
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%16.16llx", j ? " " : "",
- get_unaligned(ptr8 + j));
+ "%s%16.16llx", j ? " " : "", val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -149,9 +158,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
const u32 *ptr4 = buf;
for (j = 0; j < ngroups; j++) {
+ u32 val;
+
+ if (groupflags & DUMP_TYPE_LE)
+ val = get_unaligned_le32(ptr4 + j);
+ else if (groupflags & DUMP_TYPE_BE)
+ val = get_unaligned_be32(ptr4 + j);
+ else
+ val = get_unaligned(ptr4 + j);
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%8.8x", j ? " " : "",
- get_unaligned(ptr4 + j));
+ "%s%8.8x", j ? " " : "", val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -160,9 +176,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
const u16 *ptr2 = buf;
for (j = 0; j < ngroups; j++) {
+ u16 val;
+
+ if (groupflags & DUMP_TYPE_LE)
+ val = get_unaligned_le16(ptr2 + j);
+ else if (groupflags & DUMP_TYPE_BE)
+ val = get_unaligned_be16(ptr2 + j);
+ else
+ val = get_unaligned(ptr2 + j);
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%4.4x", j ? " " : "",
- get_unaligned(ptr2 + j));
+ "%s%4.4x", j ? " " : "", val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Andy Shevchenko <andy.shevchenko@gmail.com> |
|---|---|
| Date | 2015-12-09 21:10 +0100 |
| Message-ID | <qDTdN-4yA-23@gated-at.bofh.it> |
| In reply to | #1287767 |
On Wed, Dec 9, 2015 at 9:28 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2015-12-09 at 14:03 +0200, Andy Shevchenko wrote:
>> On Wed, Dec 9, 2015 at 2:03 AM, Joe Perches <joe@perches.com> wrote:
>> > On Tue, 2015-12-08 at 15:28 -0600, Rob Herring wrote:
>> > > On Tue, Dec 8, 2015 at 11:03 AM, Joe Perches <joe@perches.com> wrote:
>> > > > On Tue, 2015-12-08 at 08:16 -0800, Joe Perches wrote:
>> > > > > On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote:
>> > > > > > Trivial changes suggested by checkpatch.pl.
>> > > > > []
>> > > > > > diff --git a/drivers/of/address.c b/drivers/of/address.c
>> > > > > []
>> > > > > > @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev,
>> > > > > > #ifdef DEBUG
>> > > > > > static void of_dump_addr(const char *s, const __be32 *addr, int na)
>> > > > > > {
>> > > > > > - printk(KERN_DEBUG "%s", s);
>> > > > > > + pr_debug("%s", s);
>> > > > > > while (na--)
>> > > > > > printk(" %08x", be32_to_cpu(*(addr++)));
>> > > > > > printk("\n");
>> > []
>> > > > static void of_dumpaddr(const char *s, const __be32 *addr, int na)
>> > > > {
>> > > > print_hex_dump_debug(s, DUMP_PREFIX_NONE, 16, 1,
>> > > > addr, na * sizeof(__be32), false);
>> > > > }
>> > >
>> > > Except that it doesn't do the endian swapping. Looking closer at this,
>> > > we should just drop this hunk. So I will take v1.
>> >
>> > Maybe endian conversions should be added to hex_dump_debug like:
>> > (probably doesn't apply. Evolution 3.18 is horrible)
>>
>> Overall the idea is not bad I think, though few comments below.
>> Besides that, please, update test_hexdump (wrt my update to the test
>> suite [1]) to go through BE test cases.
>>
>> [1] http://www.spinics.net/lists/kernel/msg2139337.html
>
> I don't want to have to learn that code as it seems a bit tricky.
> Maybe you could do it.
Okay, I'm fine if it at least passes existing test cases. Good to
mention this in commit message then.
>
>> > 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.
> + 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.
> 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.." ?
%DUMP_…
> * @linebuf: where to put the converted data
> * @linebuflen: total size of @linebuf, including space for terminating NUL
> * @ascii: include ASCII after the hex output
> @@ -105,7 +106,7 @@ EXPORT_SYMBOL(bin2hex);
> * (excluding the terminating NUL) which would have been written to the final
> * string if enough space had been available.
> */
> -int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
> +int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupflags,
> char *linebuf, size_t linebuflen, bool ascii)
> {
> const u8 *ptr = buf;
> @@ -114,6 +115,7 @@ 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 groupsize = groupflags & ~(DUMP_TYPE_LE | DUMP_TYPE_BE);
>
> if (rowsize != 16 && rowsize != 32)
> rowsize = 16;
> @@ -138,9 +140,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
> const u64 *ptr8 = buf;
>
> for (j = 0; j < ngroups; j++) {
> + u64 val;
> +
> + if (groupflags & DUMP_TYPE_LE)
> + val = get_unaligned_le64(ptr8 + j);
> + else if (groupflags & DUMP_TYPE_BE)
> + val = get_unaligned_be64(ptr8 + j);
> + else
> + val = get_unaligned(ptr8 + j);
> ret = snprintf(linebuf + lx, linebuflen - lx,
> - "%s%16.16llx", j ? " " : "",
> - get_unaligned(ptr8 + j));
> + "%s%16.16llx", j ? " " : "", val);
> if (ret >= linebuflen - lx)
> goto overflow1;
> lx += ret;
> @@ -149,9 +158,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
> const u32 *ptr4 = buf;
>
> for (j = 0; j < ngroups; j++) {
> + u32 val;
> +
> + if (groupflags & DUMP_TYPE_LE)
> + val = get_unaligned_le32(ptr4 + j);
> + else if (groupflags & DUMP_TYPE_BE)
> + val = get_unaligned_be32(ptr4 + j);
> + else
> + val = get_unaligned(ptr4 + j);
> ret = snprintf(linebuf + lx, linebuflen - lx,
> - "%s%8.8x", j ? " " : "",
> - get_unaligned(ptr4 + j));
> + "%s%8.8x", j ? " " : "", val);
> if (ret >= linebuflen - lx)
> goto overflow1;
> lx += ret;
> @@ -160,9 +176,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
> const u16 *ptr2 = buf;
>
> for (j = 0; j < ngroups; j++) {
> + u16 val;
> +
> + if (groupflags & DUMP_TYPE_LE)
> + val = get_unaligned_le16(ptr2 + j);
> + else if (groupflags & DUMP_TYPE_BE)
> + val = get_unaligned_be16(ptr2 + j);
> + else
> + val = get_unaligned(ptr2 + j);
> ret = snprintf(linebuf + lx, linebuflen - lx,
> - "%s%4.4x", j ? " " : "",
> - get_unaligned(ptr2 + j));
> + "%s%4.4x", j ? " " : "", val);
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-12-09 21:20 +0100 |
| Subject | Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() |
| Message-ID | <qDTnr-4C7-1@gated-at.bofh.it> |
| In reply to | #1287803 |
On Wed, 2015-12-09 at 22:02 +0200, Andy Shevchenko wrote:
> On Wed, Dec 9, 2015 at 9:28 PM, Joe Perches <joe@perches.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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Rasmus Villemoes <linux@rasmusvillemoes.dk> |
|---|---|
| Date | 2015-12-09 21:20 +0100 |
| Message-ID | <qDTnt-4C7-33@gated-at.bofh.it> |
| In reply to | #1287803 |
On Wed, Dec 09 2015, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Wed, Dec 9, 2015 at 9:28 PM, Joe Perches <joe@perches.com> wrote:
>> ---
>> 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.
I think it's best to have a name for the default. In this case it's
unlikely to ever be relevant, but in general one could imagine stuff
like
#ifdef THIS_OR_THAT
#define MY_DUMP_TYPE DUMP_TYPE_LE
#else
#define MY_DUMP_TYPE DUMP_TYPE_CPU
#endif
which is a lot more readable than if the latter was a naked 0.
The feature seems fine to me. It's always been somewhat annoying that
grouping changes the order of the byte values on little-endian machines
(so grouping is actually a wrong name for it; it's more like "treat the
bytes an array of u16/u32/u64"), and one can now get around that by
specifying DUMP_TYPE_BE.
Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Andy Shevchenko <andy.shevchenko@gmail.com> |
|---|---|
| Date | 2015-12-09 21:40 +0100 |
| Message-ID | <qDTGO-4Jx-23@gated-at.bofh.it> |
| In reply to | #1287818 |
On Wed, Dec 9, 2015 at 10:17 PM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> On Wed, Dec 09 2015, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>> On Wed, Dec 9, 2015 at 9:28 PM, Joe Perches <joe@perches.com> wrote:
>>> +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.
>
> I think it's best to have a name for the default. In this case it's
> unlikely to ever be relevant, but in general one could imagine stuff
> like
>
> #ifdef THIS_OR_THAT
> #define MY_DUMP_TYPE DUMP_TYPE_LE
> #else
> #define MY_DUMP_TYPE DUMP_TYPE_CPU
> #endif
>
> which is a lot more readable than if the latter was a naked 0.
Point taken.
Though _CPU suggests user to think if it's equivalent to BE or LE. I'm
wondering if _NATIVE is better?
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-12-09 21:50 +0100 |
| Subject | Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() |
| Message-ID | <qDTQu-4PM-41@gated-at.bofh.it> |
| In reply to | #1287836 |
On Wed, 2015-12-09 at 22:36 +0200, Andy Shevchenko wrote:
[]
> wondering if _NATIVE is better?
I think CPU is better myself and it's already
in use like:
include/linux/iio/iio.h-enum iio_endian {
include/linux/iio/iio.h: IIO_CPU,
include/linux/iio/iio.h- IIO_BE,
include/linux/iio/iio.h- IIO_LE,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-12-09 23:00 +0100 |
| Subject | [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions |
| Message-ID | <qDUWe-5u5-7@gated-at.bofh.it> |
| In reply to | #1286856 |
There are use cases for dumping buffers with specific endian types
for 2, 4, and 8 byte variables in arrays.
Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such.
Rename groupsize to groupflags in the functions and add documentation
to the kernel-doc to describe the use of the DUMP_TYPE_ enum.
Signed-off-by: Joe Perches <joe@perches.com>
---
include/linux/printk.h | 23 +++++++++++++++--------
lib/hexdump.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 53 insertions(+), 19 deletions(-)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 9729565..aac2943 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -424,12 +424,19 @@ enum {
DUMP_PREFIX_ADDRESS,
DUMP_PREFIX_OFFSET
};
+
+enum {
+ DUMP_TYPE_CPU = 0,
+ 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,
+ int groupflags, char *linebuf, size_t linebuflen,
bool ascii);
#ifdef CONFIG_PRINTK
extern void print_hex_dump(const char *level, const char *prefix_str,
- int prefix_type, int rowsize, int groupsize,
+ int prefix_type, int rowsize, int groupflags,
const void *buf, size_t len, bool ascii);
#if defined(CONFIG_DYNAMIC_DEBUG)
#define print_hex_dump_bytes(prefix_str, prefix_type, buf, len) \
@@ -440,7 +447,7 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
#endif /* defined(CONFIG_DYNAMIC_DEBUG) */
#else
static inline void print_hex_dump(const char *level, const char *prefix_str,
- int prefix_type, int rowsize, int groupsize,
+ int prefix_type, int rowsize, int groupflags,
const void *buf, size_t len, bool ascii)
{
}
@@ -453,17 +460,17 @@ static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
#if defined(CONFIG_DYNAMIC_DEBUG)
#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
- groupsize, buf, len, ascii) \
+ groupflags, buf, len, ascii) \
dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
- groupsize, buf, len, ascii)
+ groupflags, buf, len, ascii)
#elif defined(DEBUG)
#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
- groupsize, buf, len, ascii) \
+ groupflags, buf, len, ascii) \
print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize, \
- groupsize, buf, len, ascii)
+ groupflags, buf, len, ascii)
#else
static inline void print_hex_dump_debug(const char *prefix_str, int prefix_type,
- int rowsize, int groupsize,
+ int rowsize, int groupflags,
const void *buf, size_t len, bool ascii)
{
}
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 992457b..f44608e 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -80,7 +80,9 @@ 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)
+ * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions
+ * of @buf, otherwise %DUMP_TYPE_CPU is used
* @linebuf: where to put the converted data
* @linebuflen: total size of @linebuf, including space for terminating NUL
* @ascii: include ASCII after the hex output
@@ -105,7 +107,7 @@ EXPORT_SYMBOL(bin2hex);
* (excluding the terminating NUL) which would have been written to the final
* string if enough space had been available.
*/
-int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
+int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupflags,
char *linebuf, size_t linebuflen, bool ascii)
{
const u8 *ptr = buf;
@@ -114,6 +116,7 @@ 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 groupsize = groupflags & ~(DUMP_TYPE_LE | DUMP_TYPE_BE);
if (rowsize != 16 && rowsize != 32)
rowsize = 16;
@@ -138,9 +141,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
const u64 *ptr8 = buf;
for (j = 0; j < ngroups; j++) {
+ u64 val;
+
+ if (groupflags & DUMP_TYPE_LE)
+ val = get_unaligned_le64(ptr8 + j);
+ else if (groupflags & DUMP_TYPE_BE)
+ val = get_unaligned_be64(ptr8 + j);
+ else
+ val = get_unaligned(ptr8 + j);
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%16.16llx", j ? " " : "",
- get_unaligned(ptr8 + j));
+ "%s%16.16llx", j ? " " : "", val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -149,9 +159,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
const u32 *ptr4 = buf;
for (j = 0; j < ngroups; j++) {
+ u32 val;
+
+ if (groupflags & DUMP_TYPE_LE)
+ val = get_unaligned_le32(ptr4 + j);
+ else if (groupflags & DUMP_TYPE_BE)
+ val = get_unaligned_be32(ptr4 + j);
+ else
+ val = get_unaligned(ptr4 + j);
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%8.8x", j ? " " : "",
- get_unaligned(ptr4 + j));
+ "%s%8.8x", j ? " " : "", val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -160,9 +177,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
const u16 *ptr2 = buf;
for (j = 0; j < ngroups; j++) {
+ u16 val;
+
+ if (groupflags & DUMP_TYPE_LE)
+ val = get_unaligned_le16(ptr2 + j);
+ else if (groupflags & DUMP_TYPE_BE)
+ val = get_unaligned_be16(ptr2 + j);
+ else
+ val = get_unaligned(ptr2 + j);
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%4.4x", j ? " " : "",
- get_unaligned(ptr2 + j));
+ "%s%4.4x", j ? " " : "", val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -216,7 +240,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
* @prefix_type: controls whether prefix of an offset, address, or none
* is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE)
* @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)
+ * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1)
+ * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions
+ * of @buf, otherwise %DUMP_TYPE_CPU is used
* @buf: data blob to dump
* @len: number of bytes in the @buf
* @ascii: include ASCII after the hex output
@@ -240,7 +267,7 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
* ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c pqrstuvwxyz{|}~.
*/
void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
- int rowsize, int groupsize,
+ int rowsize, int groupflags,
const void *buf, size_t len, bool ascii)
{
const u8 *ptr = buf;
@@ -254,7 +281,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
linelen = min(remaining, rowsize);
remaining -= rowsize;
- hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
+ hex_dump_to_buffer(ptr + i, linelen, rowsize, groupflags,
linebuf, sizeof(linebuf), ascii);
switch (prefix_type) {
--
2.6.3.368.gf34be46
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Andrew Morton <akpm@linux-foundation.org> |
|---|---|
| Date | 2015-12-09 23:10 +0100 |
| Subject | Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions |
| Message-ID | <qDV5V-5MF-31@gated-at.bofh.it> |
| In reply to | #1287908 |
On Wed, 9 Dec 2015 13:59:30 -0800 Joe Perches <joe@perches.com> wrote: > There are use cases for dumping buffers with specific endian types > for 2, 4, and 8 byte variables in arrays. > > Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such. > > Rename groupsize to groupflags in the functions and add documentation > to the kernel-doc to describe the use of the DUMP_TYPE_ enum. What are these use cases? Will patches be forthcoming to convert them? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-12-09 23:20 +0100 |
| Subject | Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions |
| Message-ID | <qDVfB-5Q2-27@gated-at.bofh.it> |
| In reply to | #1287914 |
On Wed, 2015-12-09 at 14:09 -0800, Andrew Morton wrote: > On Wed, 9 Dec 2015 13:59:30 -0800 Joe Perches <joe@perches.com> wrote: > > > There are use cases for dumping buffers with specific endian types > > for 2, 4, and 8 byte variables in arrays. > > > > Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such. > > > > Rename groupsize to groupflags in the functions and add documentation > > to the kernel-doc to describe the use of the DUMP_TYPE_ enum. > > What are these use cases? Buffers that contain LE or BE arrays that want to be emitted by print_hex_dump. > Will patches be forthcoming to convert them? Sure, after a settling period for Masahiro's patch to be applied upstream. https://lkml.org/lkml/2015/12/8/480 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Andrew Morton <akpm@linux-foundation.org> |
|---|---|
| Date | 2015-12-09 23:50 +0100 |
| Subject | Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions |
| Message-ID | <qDVIC-621-7@gated-at.bofh.it> |
| In reply to | #1287925 |
On Wed, 09 Dec 2015 14:16:36 -0800 Joe Perches <joe@perches.com> wrote: > On Wed, 2015-12-09 at 14:09 -0800, Andrew Morton wrote: > > On Wed,____9 Dec 2015 13:59:30 -0800 Joe Perches <joe@perches.com> wrote: > > > > > There are use cases for dumping buffers with specific endian types > > > for 2, 4, and 8 byte variables in arrays. > > > > > > Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such. > > > > > > Rename groupsize to groupflags in the functions and add documentation > > > to the kernel-doc to describe the use of the DUMP_TYPE_ enum. > > > > What are these use cases? > > Buffers that contain LE or BE arrays that want > to be emitted by print_hex_dump. Make that "where are". > > Will patches be forthcoming to convert them? > > Sure, after a settling period for Masahiro's patch > to be applied upstream. > > https://lkml.org/lkml/2015/12/8/480 One single callsite under #ifdef DEBUG? Doesn't seem to warrant adding code and complexity to core library functions. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-12-10 00:10 +0100 |
| Subject | Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions |
| Message-ID | <qDW1Y-6ob-17@gated-at.bofh.it> |
| In reply to | #1287951 |
On Wed, 2015-12-09 at 14:45 -0800, Andrew Morton wrote: > On Wed, 09 Dec 2015 14:16:36 -0800 Joe Perches <joe@perches.com> wrote: > > > Will patches be forthcoming to convert them? > > Sure, after a settling period for Masahiro's patch > > to be applied upstream. > > > > https://lkml.org/lkml/2015/12/8/480 > > One single callsite under #ifdef DEBUG? Doesn't seem to warrant adding > code and complexity to core library functions. It was just a for-instance. It's pretty trivial code and it's quite likely there are other buffers that could be emitted that way. e.g.: look at code like: drivers/message/fusion/mptdebug.h drivers/infiniband/hw/mthca/mthca_mr.c Dunno, I didn't look very hard. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Andy Shevchenko <andy.shevchenko@gmail.com> |
|---|---|
| Date | 2015-12-09 23:20 +0100 |
| Subject | Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions |
| Message-ID | <qDVfA-5Q2-19@gated-at.bofh.it> |
| In reply to | #1287908 |
On Wed, Dec 9, 2015 at 11:59 PM, Joe Perches <joe@perches.com> wrote:
> There are use cases for dumping buffers with specific endian types
> for 2, 4, and 8 byte variables in arrays.
>
> Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such.
>
> Rename groupsize to groupflags in the functions and add documentation
> to the kernel-doc to describe the use of the DUMP_TYPE_ enum.
One comment below and my tag
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> include/linux/printk.h | 23 +++++++++++++++--------
> lib/hexdump.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 53 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index 9729565..aac2943 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -424,12 +424,19 @@ enum {
> DUMP_PREFIX_ADDRESS,
> DUMP_PREFIX_OFFSET
> };
> +
> +enum {
> + DUMP_TYPE_CPU = 0,
> + 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,
> + int groupflags, char *linebuf, size_t linebuflen,
> bool ascii);
> #ifdef CONFIG_PRINTK
> extern void print_hex_dump(const char *level, const char *prefix_str,
> - int prefix_type, int rowsize, int groupsize,
> + int prefix_type, int rowsize, int groupflags,
> const void *buf, size_t len, bool ascii);
> #if defined(CONFIG_DYNAMIC_DEBUG)
> #define print_hex_dump_bytes(prefix_str, prefix_type, buf, len) \
> @@ -440,7 +447,7 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
> #endif /* defined(CONFIG_DYNAMIC_DEBUG) */
> #else
> static inline void print_hex_dump(const char *level, const char *prefix_str,
> - int prefix_type, int rowsize, int groupsize,
> + int prefix_type, int rowsize, int groupflags,
> const void *buf, size_t len, bool ascii)
> {
> }
> @@ -453,17 +460,17 @@ static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
>
> #if defined(CONFIG_DYNAMIC_DEBUG)
> #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
> - groupsize, buf, len, ascii) \
> + groupflags, buf, len, ascii) \
> dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
> - groupsize, buf, len, ascii)
> + groupflags, buf, len, ascii)
> #elif defined(DEBUG)
> #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
> - groupsize, buf, len, ascii) \
> + groupflags, buf, len, ascii) \
> print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize, \
> - groupsize, buf, len, ascii)
> + groupflags, buf, len, ascii)
> #else
> static inline void print_hex_dump_debug(const char *prefix_str, int prefix_type,
> - int rowsize, int groupsize,
> + int rowsize, int groupflags,
> const void *buf, size_t len, bool ascii)
> {
> }
> diff --git a/lib/hexdump.c b/lib/hexdump.c
> index 992457b..f44608e 100644
> --- a/lib/hexdump.c
> +++ b/lib/hexdump.c
> @@ -80,7 +80,9 @@ 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)
> + * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions
> + * of @buf, otherwise %DUMP_TYPE_CPU is used
> * @linebuf: where to put the converted data
> * @linebuflen: total size of @linebuf, including space for terminating NUL
> * @ascii: include ASCII after the hex output
> @@ -105,7 +107,7 @@ EXPORT_SYMBOL(bin2hex);
> * (excluding the terminating NUL) which would have been written to the final
> * string if enough space had been available.
> */
> -int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
> +int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupflags,
> char *linebuf, size_t linebuflen, bool ascii)
> {
> const u8 *ptr = buf;
> @@ -114,6 +116,7 @@ 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 groupsize = groupflags & ~(DUMP_TYPE_LE | DUMP_TYPE_BE);
>
> if (rowsize != 16 && rowsize != 32)
> rowsize = 16;
> @@ -138,9 +141,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
> const u64 *ptr8 = buf;
>
> for (j = 0; j < ngroups; j++) {
> + u64 val;
> +
> + if (groupflags & DUMP_TYPE_LE)
> + val = get_unaligned_le64(ptr8 + j);
> + else if (groupflags & DUMP_TYPE_BE)
> + val = get_unaligned_be64(ptr8 + j);
> + else
> + val = get_unaligned(ptr8 + j);
> ret = snprintf(linebuf + lx, linebuflen - lx,
> - "%s%16.16llx", j ? " " : "",
> - get_unaligned(ptr8 + j));
> + "%s%16.16llx", j ? " " : "", val);
> if (ret >= linebuflen - lx)
> goto overflow1;
> lx += ret;
> @@ -149,9 +159,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
> const u32 *ptr4 = buf;
>
> for (j = 0; j < ngroups; j++) {
> + u32 val;
> +
> + if (groupflags & DUMP_TYPE_LE)
> + val = get_unaligned_le32(ptr4 + j);
> + else if (groupflags & DUMP_TYPE_BE)
> + val = get_unaligned_be32(ptr4 + j);
> + else
> + val = get_unaligned(ptr4 + j);
> ret = snprintf(linebuf + lx, linebuflen - lx,
> - "%s%8.8x", j ? " " : "",
> - get_unaligned(ptr4 + j));
> + "%s%8.8x", j ? " " : "", val);
> if (ret >= linebuflen - lx)
> goto overflow1;
> lx += ret;
> @@ -160,9 +177,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
> const u16 *ptr2 = buf;
>
> for (j = 0; j < ngroups; j++) {
> + u16 val;
> +
> + if (groupflags & DUMP_TYPE_LE)
> + val = get_unaligned_le16(ptr2 + j);
> + else if (groupflags & DUMP_TYPE_BE)
> + val = get_unaligned_be16(ptr2 + j);
> + else
> + val = get_unaligned(ptr2 + j);
> ret = snprintf(linebuf + lx, linebuflen - lx,
> - "%s%4.4x", j ? " " : "",
> - get_unaligned(ptr2 + j));
> + "%s%4.4x", j ? " " : "", val);
> if (ret >= linebuflen - lx)
> goto overflow1;
> lx += ret;
> @@ -216,7 +240,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
> * @prefix_type: controls whether prefix of an offset, address, or none
> * is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE)
> * @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)
> + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1)
Duplicate.
> + * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions
> + * of @buf, otherwise %DUMP_TYPE_CPU is used
> * @buf: data blob to dump
> * @len: number of bytes in the @buf
> * @ascii: include ASCII after the hex output
> @@ -240,7 +267,7 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
> * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c pqrstuvwxyz{|}~.
> */
> void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
> - int rowsize, int groupsize,
> + int rowsize, int groupflags,
> const void *buf, size_t len, bool ascii)
> {
> const u8 *ptr = buf;
> @@ -254,7 +281,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
> linelen = min(remaining, rowsize);
> remaining -= rowsize;
>
> - hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
> + hex_dump_to_buffer(ptr + i, linelen, rowsize, groupflags,
> linebuf, sizeof(linebuf), ascii);
>
> switch (prefix_type) {
> --
> 2.6.3.368.gf34be46
>
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-12-09 23:30 +0100 |
| Subject | Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions |
| Message-ID | <qDVpg-5UZ-15@gated-at.bofh.it> |
| In reply to | #1287921 |
On Thu, 2015-12-10 at 00:11 +0200, Andy Shevchenko wrote: > On Wed, Dec 9, 2015 at 11:59 PM, Joe Perches <joe@perches.com> wrote: [] > > diff --git a/lib/hexdump.c b/lib/hexdump.c [] > > @@ -216,7 +240,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer); > > * @prefix_type: controls whether prefix of an offset, address, or none > > * is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE) > > * @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) > > + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) > > Duplicate. Ick. Andrew, can you delete one line please or do you want a resend? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Andy Shevchenko <andy.shevchenko@gmail.com> |
|---|---|
| Date | 2015-12-10 00:10 +0100 |
| Subject | Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions |
| Message-ID | <qDW21-6ob-73@gated-at.bofh.it> |
| In reply to | #1287921 |
On Thu, Dec 10, 2015 at 12:11 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Dec 9, 2015 at 11:59 PM, Joe Perches <joe@perches.com> wrote:
>> There are use cases for dumping buffers with specific endian types
>> for 2, 4, and 8 byte variables in arrays.
>>
>> Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such.
>>
>> Rename groupsize to groupflags in the functions and add documentation
>> to the kernel-doc to describe the use of the DUMP_TYPE_ enum.
>
> One comment below and my tag
> Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Couple of additional notes. (Tag still in power)
1. There is similar to print_hex_dump() function in seq_file. Separate patch?
2. What to do with ASCII part? Shall we print it as byte stream?
And one more style comment below.
>
>>
>> Signed-off-by: Joe Perches <joe@perches.com>
>> ---
>> include/linux/printk.h | 23 +++++++++++++++--------
>> lib/hexdump.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
>> 2 files changed, 53 insertions(+), 19 deletions(-)
>>
>> diff --git a/include/linux/printk.h b/include/linux/printk.h
>
>> index 9729565..aac2943 100644
>> --- a/include/linux/printk.h
>> +++ b/include/linux/printk.h
>> @@ -424,12 +424,19 @@ enum {
>> DUMP_PREFIX_ADDRESS,
>> DUMP_PREFIX_OFFSET
>> };
>> +
>> +enum {
>> + DUMP_TYPE_CPU = 0,
>> + 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,
>> + int groupflags, char *linebuf, size_t linebuflen,
>> bool ascii);
>> #ifdef CONFIG_PRINTK
>> extern void print_hex_dump(const char *level, const char *prefix_str,
>> - int prefix_type, int rowsize, int groupsize,
>> + int prefix_type, int rowsize, int groupflags,
>> const void *buf, size_t len, bool ascii);
>> #if defined(CONFIG_DYNAMIC_DEBUG)
>> #define print_hex_dump_bytes(prefix_str, prefix_type, buf, len) \
>> @@ -440,7 +447,7 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
>> #endif /* defined(CONFIG_DYNAMIC_DEBUG) */
>> #else
>> static inline void print_hex_dump(const char *level, const char *prefix_str,
>> - int prefix_type, int rowsize, int groupsize,
>> + int prefix_type, int rowsize, int groupflags,
>> const void *buf, size_t len, bool ascii)
>> {
>> }
>> @@ -453,17 +460,17 @@ static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
>>
>> #if defined(CONFIG_DYNAMIC_DEBUG)
>> #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
>> - groupsize, buf, len, ascii) \
>> + groupflags, buf, len, ascii) \
>> dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
>> - groupsize, buf, len, ascii)
>> + groupflags, buf, len, ascii)
>> #elif defined(DEBUG)
>> #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
>> - groupsize, buf, len, ascii) \
>> + groupflags, buf, len, ascii) \
>> print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize, \
>> - groupsize, buf, len, ascii)
>> + groupflags, buf, len, ascii)
>> #else
>> static inline void print_hex_dump_debug(const char *prefix_str, int prefix_type,
>> - int rowsize, int groupsize,
>> + int rowsize, int groupflags,
>> const void *buf, size_t len, bool ascii)
>> {
>> }
>> diff --git a/lib/hexdump.c b/lib/hexdump.c
>> index 992457b..f44608e 100644
>> --- a/lib/hexdump.c
>> +++ b/lib/hexdump.c
>> @@ -80,7 +80,9 @@ 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)
>> + * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions
>> + * of @buf, otherwise %DUMP_TYPE_CPU is used
>> * @linebuf: where to put the converted data
>> * @linebuflen: total size of @linebuf, including space for terminating NUL
>> * @ascii: include ASCII after the hex output
>> @@ -105,7 +107,7 @@ EXPORT_SYMBOL(bin2hex);
>> * (excluding the terminating NUL) which would have been written to the final
>> * string if enough space had been available.
>> */
>> -int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
>> +int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupflags,
>> char *linebuf, size_t linebuflen, bool ascii)
>> {
>> const u8 *ptr = buf;
>> @@ -114,6 +116,7 @@ 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 groupsize = groupflags & ~(DUMP_TYPE_LE | DUMP_TYPE_BE);
Can you move this upper to first or second position in the definition block?
>>
>> if (rowsize != 16 && rowsize != 32)
>> rowsize = 16;
>> @@ -138,9 +141,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
>> const u64 *ptr8 = buf;
>>
>> for (j = 0; j < ngroups; j++) {
>> + u64 val;
>> +
>> + if (groupflags & DUMP_TYPE_LE)
>> + val = get_unaligned_le64(ptr8 + j);
>> + else if (groupflags & DUMP_TYPE_BE)
>> + val = get_unaligned_be64(ptr8 + j);
>> + else
>> + val = get_unaligned(ptr8 + j);
>> ret = snprintf(linebuf + lx, linebuflen - lx,
>> - "%s%16.16llx", j ? " " : "",
>> - get_unaligned(ptr8 + j));
>> + "%s%16.16llx", j ? " " : "", val);
>> if (ret >= linebuflen - lx)
>> goto overflow1;
>> lx += ret;
>> @@ -149,9 +159,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
>> const u32 *ptr4 = buf;
>>
>> for (j = 0; j < ngroups; j++) {
>> + u32 val;
>> +
>> + if (groupflags & DUMP_TYPE_LE)
>> + val = get_unaligned_le32(ptr4 + j);
>> + else if (groupflags & DUMP_TYPE_BE)
>> + val = get_unaligned_be32(ptr4 + j);
>> + else
>> + val = get_unaligned(ptr4 + j);
>> ret = snprintf(linebuf + lx, linebuflen - lx,
>> - "%s%8.8x", j ? " " : "",
>> - get_unaligned(ptr4 + j));
>> + "%s%8.8x", j ? " " : "", val);
>> if (ret >= linebuflen - lx)
>> goto overflow1;
>> lx += ret;
>> @@ -160,9 +177,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
>> const u16 *ptr2 = buf;
>>
>> for (j = 0; j < ngroups; j++) {
>> + u16 val;
>> +
>> + if (groupflags & DUMP_TYPE_LE)
>> + val = get_unaligned_le16(ptr2 + j);
>> + else if (groupflags & DUMP_TYPE_BE)
>> + val = get_unaligned_be16(ptr2 + j);
>> + else
>> + val = get_unaligned(ptr2 + j);
>> ret = snprintf(linebuf + lx, linebuflen - lx,
>> - "%s%4.4x", j ? " " : "",
>> - get_unaligned(ptr2 + j));
>> + "%s%4.4x", j ? " " : "", val);
>> if (ret >= linebuflen - lx)
>> goto overflow1;
>> lx += ret;
>> @@ -216,7 +240,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
>> * @prefix_type: controls whether prefix of an offset, address, or none
>> * is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE)
>> * @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)
>
>> + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1)
>
> Duplicate.
>
>> + * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions
>> + * of @buf, otherwise %DUMP_TYPE_CPU is used
>> * @buf: data blob to dump
>> * @len: number of bytes in the @buf
>> * @ascii: include ASCII after the hex output
>> @@ -240,7 +267,7 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
>> * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c pqrstuvwxyz{|}~.
>> */
>> void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>> - int rowsize, int groupsize,
>> + int rowsize, int groupflags,
>> const void *buf, size_t len, bool ascii)
>> {
>> const u8 *ptr = buf;
>> @@ -254,7 +281,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>> linelen = min(remaining, rowsize);
>> remaining -= rowsize;
>>
>> - hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
>> + hex_dump_to_buffer(ptr + i, linelen, rowsize, groupflags,
>> linebuf, sizeof(linebuf), ascii);
>>
>> switch (prefix_type) {
>> --
>> 2.6.3.368.gf34be46
>>
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web