lib: add ascii hex helper functions
PATCH 1/4 lib: add ascii hex helper functions
Everyone rolls their own version around the tree, centralize in lib/hexdump.c
--- include/linux/kernel.h | 6 +++++- lib/hexdump.c | 25 +++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-)
include/linux/kernel.h +5 -1
@@ -276,7 +276,11 @@ extern void print_hex_dump(const char *level, const char *prefix_str, const void *buf, size_t len, bool ascii);extern void print_hex_dump_bytes (const char *prefix_str,int prefix_type,const void *buf,size_t len); -#define hex_asc (x) "0123456789abcdef"[x] + +extern const char hex_asc[]; +extern int hex_to_int (char c); +#define hex_asc_lo (x) hex_asc[((x) & 0x0f)] +#define hex_asc_hi (x) hex_asc[((x) & 0xf0) >> 4] #define pr_emerg(fmt, arg...) \ printk(KERN_EMERG fmt, ##arg)
lib/hexdump.c +23 -2
@@ -12,6 +12,27 @@#include <linux/kernel.h> #include <linux/module.h> +const char hexasc[16 ] ="0123456789abcdef" ; + +/** +* hex_to_int - convert a single hex ASCII char to an integer value +* @ch: char to convert, not case sensitive [a-f][A-F][0-9] +* +* Returns -1 if the char is not a valid hexadecimal character +*/ +int hex_to_int (char ch) +{ +/* +* Make ch lower-case, works only for digits and letters +*/ + ch |=0x20 ; +if (ch >='a' && ch <='f' ) +return ch -'a' +10 ; +if (ch >='0' && ch <='9' ) +return ch -'0' ; +return -1 ; +} +/** * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory * @buf: data blob to dump
@@ -93,8 +114,8 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,for (j =0 ; (j < rowsize) && (j < len) && (lx +4 ) < linebuflen; j++) { ch = ptr[j]; - linebuf[lx++] =hex_asc (ch >>4 ); - linebuf[lx++] =hex_asc (ch &0x0f ); + linebuf[lx++] =hex_asc_hi (ch); + linebuf[lx++] =hex_asc_lo (ch); linebuf[lx++] =' ' ; } ascii_column =3 * rowsize +2 ;
Signature · 1 line
1.5.5.1.305.g7c84
Andrew Morton
maintainerReview
Quoted 5 lines · On Thu, 01 May 2008 13:51:07 -0700 Harvey Harrison <[email protected]>…
Everyone rolls their own version around the tree, centralize in
lib/hexdump.c(quoted diff) +2
-0+extern const char hex_asc[]; +const char hexasc[16] = "0123456789abcdef";
If I were a compiler I'd get upset about that.
Honest, 24 hours really doesn't matter here. Do it carefully, do it right.
Quoted 6 lines · Andrew Morton
Quoted 3 lines · On Thu, 01 May 2008 13:51:07 -0700 Harvey Harrison <[email protected]>…
Everyone rolls their own version around the tree, centralize in
lib/hexdump.c
Quoted 7 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+extern const char hex_asc[]; +const char hexasc[16] = "0123456789abcdef";If I were a compiler I'd get upset about that.
Honest, 24 hours really doesn't matter here. Do it carefully, do it right.
...insert something here about improving quality ;-)
Harvey
Quoted 14 lines · Harvey Harrison
Everyone rolls their own version around the tree, centralize in
lib/hexdump.cSigned-off-by Harvey Harrison <[email protected]>--- include/linux/kernel.h | 6 +++++- lib/hexdump.c | 25 +++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-)include/linux/kernel.h +0
-0
include/linux/kernel.h extern void print_hex_dump(const char *level, const char *prefix_str, · line 280
Quoted 6 lines
include/linux/kernel.h +2
-1@@ -276,7 +276,11 @@ extern void print_hex_dump(const char *level, const char *prefix_str, const void *buf, size_t len, bool ascii);extern void print_hex_dump_bytes (const char *prefix_str,int prefix_type,const void *buf,size_t len); -#define hex_asc (x) "0123456789abcdef"[x] + +extern const char hex_asc[];
I don't see a reason hex_asc should be extern.
It's only used by hexdump.
I think it better to remove hex_asc from kernel.h altogether.
Quoted 10 lines · Joe Perches
Quoted 9 lines · Harvey Harrison
Everyone rolls their own version around the tree, centralize in
lib/hexdump.cSigned-off-by Harvey Harrison <[email protected]>--- include/linux/kernel.h | 6 +++++- lib/hexdump.c | 25 +++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-)
Quoted 17 lines
include/linux/kernel.h extern void print_hex_dump(const char *level, const char *prefix_str, · line 280
Quoted 11 lines
include/linux/kernel.h +2
-1@@ -276,7 +276,11 @@ extern void print_hex_dump(const char *level, const char *prefix_str, const void *buf, size_t len, bool ascii);extern void print_hex_dump_bytes (const char *prefix_str,int prefix_type,const void *buf,size_t len); -#define hex_asc (x) "0123456789abcdef"[x] + +extern const char hex_asc[];I don't see a reason
hex_ascshould be extern.
It's only used by hexdump.
I think it better to removehex_ascfromkernel.haltogether.
The whole point is to start to centralize everybody's private copy of a hex_asc equivalent.
Harvey
Quoted 2 lines · Harvey Harrison
The whole point is to start to centralize everybody's private copy of a
hex_ascequivalent.
Which should be done via some function,
not via a char array right?
Quoted 7 lines · Joe Perches
Quoted 2 lines · Harvey Harrison
The whole point is to start to centralize everybody's private copy of a
hex_ascequivalent.Which should be done via some function,
not via a char array right?
It's just getting a char from the array, a static inline maybe rather than the hex_asc_hi/lo macros, but the array would still need to be extern so the inlines could get at it. The whole point is avoiding a function call...which is why everybody has their own static array they can use directly in the first place.
Harvey