Skip to contentSkip to conversation

fs: use kernel's hex_to_bin() method

PATCH RESEND In review Andy Shevchenko 3 participants 6 messages Dec 10, 2010 – Jan 5, 2011

Subsystems (3)
VFS Library code exec and binfmt
lore
Share & tools
lore

Andy Shevchenko

author
#0 lore raw

PATCH RESEND 1/1 fs: use kernel's hex_to_bin() method

Signed-off-by Andy Shevchenko <[email protected]>
Cc Alexander Viro <[email protected]>
Cc [email protected]
---
 fs/binfmt_misc.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)
fs/binfmt_misc.c +3 -4
index 1befe2e..a0fb271 100644
@@ -28,6 +28,7 @@
 #include <linux/mount.h>
 #include <linux/syscalls.h>
 #include <linux/fs.h>
+#include <linux/kernel.h>
 
 #include <asm/uaccess.h>
 

@@ -244,10 +245,8 @@ static int unquote(char *from)
 	while ((c = *s++) != '\0') {
 		if (c == '\\' && *s == 'x') {
 			s++;
-			c = toupper(*s++);
-			*p = (c - (isdigit(c) ? '0' : 'A' - 10)) << 4;
-			c = toupper(*s++);
-			*p++ |= c - (isdigit(c) ? '0' : 'A' - 10);
+			*p = hex_to_bin(*s++) << 4;
+			*p++ |= hex_to_bin(*s++);
 			continue;
 		}
 		*p++ = c;
Signature · 1 line
1.7.2.3
maintainer
#1 lore raw
Quoted 22 lines · On Fri, 10 Dec 2010 12:55:05 +0200 Andy Shevchenko <[email protected]>…
Signed-off-by Andy Shevchenko <[email protected]>
Cc Alexander Viro <[email protected]>
Cc [email protected]
---
 fs/binfmt_misc.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)
fs/binfmt_misc.c +1 -0
index 1befe2e..a0fb271 100644
@@ -28,6 +28,7 @@
 #include <linux/mount.h>
 #include <linux/syscalls.h>
 #include <linux/fs.h>
+#include <linux/kernel.h>
 
 #include <asm/uaccess.h>
 

@@ -244,10 +245,8 @@ static int unquote(char *from)
 	while ((c = *s++) != '\0') {
 		if (c == '\\' && *s == 'x') {

fs/binfmt_misc.c static int unquote(char *from) · line 252

Quoted 10 lines
fs/binfmt_misc.c +2 -4
 			s++;
-			c = toupper(*s++);
-			*p = (c - (isdigit(c) ? '0' : 'A' - 10)) << 4;
-			c = toupper(*s++);
-			*p++ |= c - (isdigit(c) ? '0' : 'A' - 10);
+			*p = hex_to_bin(*s++) << 4;
+			*p++ |= hex_to_bin(*s++);
 			continue;
 		}
 		*p++ = c;

Calling hex_to_bin() twice is a bit sad - we need a library function which does hex-to-bin on a digit string.

And lo, one just got added in linux-next. However it cruelly didn't return anything useful, so...

From: Andrew Morton <[email protected]>

As a convenience to callers.

Not a terribly convenient convenience, as most callers will need to cast away the constness of the return value. Oh well, those callers should have been using a `const char *' anyway.

Cc Mimi Zohar <[email protected]>
Cc Serge E. Hallyn <[email protected]>
Cc David Howells <[email protected]>
Cc James Morris <[email protected]>
Cc Andy Shevchenko <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

---

 include/linux/kernel.h |    2 +-
 lib/hexdump.c          |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)
lib/hexdump.c renamed from lib/hexdump.c~lib-hexdumpc-make-hex2bin-return-the-updated-src-address +2 -1
diff -puN lib/hexdump.c~lib-hexdumpc-make-hex2bin-return-the-updated-src-address lib/hexdump.c
@@ -39,13 +39,14 @@ EXPORT_SYMBOL(hex_to_bin);
  * @src: ascii hexadecimal string
  * @count: result length
  */
-void hex2bin(u8 *dst, const char *src, size_t count)
+const char *hex2bin(u8 *dst, const char *src, size_t count)
 {
 	while (count--) {
 		*dst = hex_to_bin(*src++) << 4;
 		*dst += hex_to_bin(*src++);
 		dst++;
 	}
+	return src;
 }
 EXPORT_SYMBOL(hex2bin);
include/linux/kernel.h renamed from include/linux/kernel.h~lib-hexdumpc-make-hex2bin-return-the-updated-src-address +1 -1
diff -puN include/linux/kernel.h~lib-hexdumpc-make-hex2bin-return-the-updated-src-address include/linux/kernel.h
@@ -278,7 +278,7 @@ static inline char *pack_hex_byte(char *
 }
 
 extern int hex_to_bin(char ch);
-extern void hex2bin(u8 *dst, const char *src, size_t count);
+extern const char *hex2bin(u8 *dst, const char *src, size_t count);
 
 /*
  * General tracing related utility functions - trace_printk(),

_

After which we can change your patch thusly:

fs/binfmt_misc.c renamed from fs/binfmt_misc.c~fs-binfmt_miscc-use-kernels-hex_to_bin-method-fix +1 -3
@@ -244,9 +244,7 @@ static int unquote(char *from)
 
 	while ((c = *s++) != '\0') {
 		if (c == '\\' && *s == 'x') {
-			s++;
-			*p = hex_to_bin(*s++) << 4;
-			*p++ |= hex_to_bin(*s++);
+			s = (char *)hex2bin(p, s + 1, 1);
 			continue;
 		}
 		*p++ = c;

_

author
#2 lore raw

fs/binfmt_misc.c static int unquote(char *from) · line 247

Quoted 12 lines · Andrew Morton

After which we can change your patch thusly:

fs/binfmt_misc.c renamed from fs/binfmt_misc.c~fs-binfmt_miscc-use-kernels-hex_to_bin-method-fix +1 -3
@@ -244,9 +244,7 @@ static int unquote(char *from)
 
        while ((c = *s++) != '\0') {
                if (c == '\\' && *s == 'x') {
-                       s++;
-                       *p = hex_to_bin(*s++) << 4;
-                       *p++ |= hex_to_bin(*s++);
+                       s = (char *)hex2bin(p, s + 1, 1);

We need to update both p and s.

In case of '\xAA\xBB' you have result -> '0xBB'.
Am I wrong?

Quoted 3 lines
fs/binfmt_misc.c renamed from fs/binfmt_misc.c~fs-binfmt_miscc-use-kernels-hex_to_bin-method-fix +0 -0
                        continue;
                }
                *p++ = c;
Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#3 lore raw
Quoted 16 lines · On Wed, 5 Jan 2011 01:58:56 +0200 Andy Shevchenko <[email protected]>…

fs/binfmt_misc.c static int unquote(char *from) · line 247

Quoted 12 lines · Andrew Morton

After which we can change your patch thusly:

fs/binfmt_misc.c renamed from fs/binfmt_misc.c~fs-binfmt_miscc-use-kernels-hex_to_bin-method-fix +1 -3
@@ -244,9 +244,7 @@ static int unquote(char *from)
 
       while ((c = *s++) != '\0') {
               if (c == '\\' && *s == 'x') {
-                       s++;
-                       *p = hex_to_bin(*s++) << 4;
-                       *p++ |= hex_to_bin(*s++);
+                       s = (char *)hex2bin(p, s + 1, 1);

We need to update both p and s.

Quoted 6 lines

In case of '\xAA\xBB' you have result -> '0xBB'.
Am I wrong?

Quoted 3 lines
fs/binfmt_misc.c renamed from fs/binfmt_misc.c~fs-binfmt_miscc-use-kernels-hex_to_bin-method-fix +0 -0
                       continue;
               }
               *p++ = c;

I'm more into shopping and nagging at people than this programming thingy. Sigh.

fs/binfmt_misc.c renamed from fs/binfmt_misc.c~fs-binfmt_miscc-use-kernels-hex_to_bin-method-fix-fix +4 -5
@@ -243,11 +243,10 @@ static int unquote(char *from)
 	char c = 0, *s = from, *p = from;
 
 	while ((c = *s++) != '\0') {
-		if (c == '\\' && *s == 'x') {
-			s = (char *)hex2bin(p, s + 1, 1);
-			continue;
-		}
-		*p++ = c;
+		if (c == '\\' && *s == 'x')
+			s = (char *)hex2bin(p++, s + 1, 1);
+		else
+			*p++ = c;
 	}
 	return p - from;
 }

_

How does one test this thing?

#4 lore raw
Quoted 3 lines · Andrew Morton

How does one test this thing?

From userspace.

David

#5 lore raw
In reply to Andrew Morton
Quoted 3 lines · Andrew Morton

Not a terribly convenient convenience, as most callers will need to cast away the constness of the return value. Oh well, those callers should have been using a `const char *' anyway.

Maybe. You could always do as strchr():

	char *strchr(const char *s, int c);

Then you don't need to cast the result. The problem[*] is that you can't tell the compiler that the return value should carry the same constness as one of the arguments because you don't have something like C++ templates.

[*] If indeed not having C++ or templates should be seen as a problem...

David