Re: [PATCH 4/5] fs: fat: use hex_asc_lo/hex_asc_hi instead of custom one

Re: [PATCH 4/5] fs: fat: use hex_asc_lo/hex_asc_hi instead of custom one

[OGAWA Hirofumi]

Andy Shevchenko <andy.shevchenko@gmail.com> writes:

>  			if (uni_xlate == 1) {
> -				*op = ':';
> -				for (k = 4; k > 0; k--) {
> -					nc = ec & 0xF;
> -					op[k] = nc > 9	? nc + ('a' - 10)
> -							: nc + '0';
> -					ec >>= 4;
> -				}
> -				op += 5;
> +				*op++ = ':';
> +				*op++ = hex_asc_hi(ec >> 8);
> +				*op++ = hex_asc_lo(ec >> 8);
> +				*op++ = hex_asc_hi(ec);
> +				*op++ = hex_asc_lo(ec);
>  				len -= 5;

Why doesn't this use pack_hex_byte()?

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH 4/5] fs: fat: use hex_asc_lo/hex_asc_hi instead of custom one

[Andy Shevchenko]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 678 bytes --]

On Thu, Mar 11, 2010 at 1:18 AM, OGAWA Hirofumi<hirofumi@mail.parknet.co.jp> wrote:>> +                             *op++ = hex_asc_hi(ec >> 8);>> +                             *op++ = hex_asc_lo(ec >> 8);>> +                             *op++ = hex_asc_hi(ec);>> +                             *op++ = hex_asc_lo(ec);> Why doesn't this use pack_hex_byte()?No specific reason, it could be so. Would you like me to change this?
-- With Best Regards,Andy Shevchenkoÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 4/5] fs: fat: use hex_asc_lo/hex_asc_hi instead of custom one

[OGAWA Hirofumi]

Andy Shevchenko <andy.shevchenko@gmail.com> writes:

> On Thu, Mar 11, 2010 at 1:18 AM, OGAWA Hirofumi
> <hirofumi@mail.parknet.co.jp> wrote:
>>> +                             *op++ = hex_asc_hi(ec >> 8);
>>> +                             *op++ = hex_asc_lo(ec >> 8);
>>> +                             *op++ = hex_asc_hi(ec);
>>> +                             *op++ = hex_asc_lo(ec);
>> Why doesn't this use pack_hex_byte()?
> No specific reason, it could be so. Would you like me to change this?

Yes, please change it if you have no problem.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[PATCHv2] fs: fat: use pack_hex_byte() instead of custom one

[Andy Shevchenko]

From: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 fs/fat/dir.c |   14 +++++---------
 1 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 530b4ca..5de1a70 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -19,6 +19,7 @@
 #include <linux/buffer_head.h>
 #include <linux/compat.h>
 #include <asm/uaccess.h>
+#include <linux/kernel.h>
 #include "fat.h"
 
 /*
@@ -149,19 +150,14 @@ static int uni16_to_x8(unsigned char *ascii, const wchar_t *uni, int len,
 
 	while (*ip && ((len - NLS_MAX_CHARSET_SIZE) > 0)) {
 		ec = *ip++;
-		if ( (charlen = nls->uni2char(ec, op, NLS_MAX_CHARSET_SIZE)) > 0) {
+		if ((charlen = nls->uni2char(ec, op, NLS_MAX_CHARSET_SIZE)) > 0) {
 			op += charlen;
 			len -= charlen;
 		} else {
 			if (uni_xlate == 1) {
-				*op = ':';
-				for (k = 4; k > 0; k--) {
-					nc = ec & 0xF;
-					op[k] = nc > 9	? nc + ('a' - 10)
-							: nc + '0';
-					ec >>= 4;
-				}
-				op += 5;
+				*op++ = ':';
+				op = pack_hex_byte(op, ec >> 8);
+				op = pack_hex_byte(op, ec);
 				len -= 5;
 			} else {
 				*op++ = '?';
-- 
1.5.6.5

Re: [PATCHv2] fs: fat: use pack_hex_byte() instead of custom one

[OGAWA Hirofumi]

Andy Shevchenko <andy.shevchenko@gmail.com> writes:

> From: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
>
> Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>

Looks good to me.

	Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

(From 1/4 with previous patch, I guess it's series of patches) Or I
should take this patch into my tree?

Thanks.

> ---
>  fs/fat/dir.c |   14 +++++---------
>  1 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index 530b4ca..5de1a70 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -19,6 +19,7 @@
>  #include <linux/buffer_head.h>
>  #include <linux/compat.h>
>  #include <asm/uaccess.h>
> +#include <linux/kernel.h>
>  #include "fat.h"
>  
>  /*
> @@ -149,19 +150,14 @@ static int uni16_to_x8(unsigned char *ascii, const wchar_t *uni, int len,
>  
>  	while (*ip && ((len - NLS_MAX_CHARSET_SIZE) > 0)) {
>  		ec = *ip++;
> -		if ( (charlen = nls->uni2char(ec, op, NLS_MAX_CHARSET_SIZE)) > 0) {
> +		if ((charlen = nls->uni2char(ec, op, NLS_MAX_CHARSET_SIZE)) > 0) {
>  			op += charlen;
>  			len -= charlen;
>  		} else {
>  			if (uni_xlate == 1) {
> -				*op = ':';
> -				for (k = 4; k > 0; k--) {
> -					nc = ec & 0xF;
> -					op[k] = nc > 9	? nc + ('a' - 10)
> -							: nc + '0';
> -					ec >>= 4;
> -				}
> -				op += 5;
> +				*op++ = ':';
> +				op = pack_hex_byte(op, ec >> 8);
> +				op = pack_hex_byte(op, ec);
>  				len -= 5;
>  			} else {
>  				*op++ = '?';

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCHv2] fs: fat: use pack_hex_byte() instead of custom one

[Andy Shevchenko]

On Thu, Mar 11, 2010 at 5:12 PM, OGAWA Hirofumi
<hirofumi@mail.parknet.co.jp> wrote:
> (From 1/4 with previous patch, I guess it's series of patches) Or I
> should take this patch into my tree?
It has only one in common - cleanup. Basically it's a bunch of
independent fixes.
Please, take it in your tree. thank you.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCHv3] fs: fat: use pack_hex_byte() instead of custom one

[OGAWA Hirofumi]

Andy Shevchenko <andy.shevchenko@gmail.com> writes:

>  	const wchar_t *ip;
>  	wchar_t ec;
> -	unsigned char *op, nc;
> +	unsigned char *op;
>  	int charlen;
> -	int k;

Ah, thanks. I fixed this myself before commit.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH 4/5] fs: fat: use hex_asc_lo/hex_asc_hi instead of custom one

[Joe Perches]

On Thu, 2010-03-11 at 08:18 +0900, OGAWA Hirofumi wrote:
> Andy Shevchenko <andy.shevchenko@gmail.com> writes:
> 
> >  			if (uni_xlate == 1) {
> > -				*op = ':';
> > -				for (k = 4; k > 0; k--) {
> > -					nc = ec & 0xF;
> > -					op[k] = nc > 9	? nc + ('a' - 10)
> > -							: nc + '0';
> > -					ec >>= 4;
> > -				}
> > -				op += 5;
> > +				*op++ = ':';
> > +				*op++ = hex_asc_hi(ec >> 8);
> > +				*op++ = hex_asc_lo(ec >> 8);
> > +				*op++ = hex_asc_hi(ec);
> > +				*op++ = hex_asc_lo(ec);
> >  				len -= 5;
> 
> Why doesn't this use pack_hex_byte()?

or snprintf

Re: [PATCH 4/5] fs: fat: use hex_asc_lo/hex_asc_hi instead of custom one

[Andy Shevchenko]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 638 bytes --]

On Thu, Mar 11, 2010 at 9:21 AM, Joe Perches <joe@perches.com> wrote:>> > +                           *op++ = hex_asc_hi(ec >> 8);>> > +                           *op++ = hex_asc_lo(ec >> 8);>> > +                           *op++ = hex_asc_hi(ec);>> > +                           *op++ = hex_asc_lo(ec);>> Why doesn't this use pack_hex_byte()?> or snprintfsprintf looks like overkill here.
-- With Best Regards,Andy Shevchenkoÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 4/5] fs: fat: use hex_asc_lo/hex_asc_hi instead of custom one

[Joe Perches]

On Thu, 2010-03-11 at 09:59 +0200, Andy Shevchenko wrote:
> On Thu, Mar 11, 2010 at 9:21 AM, Joe Perches <joe@perches.com> wrote:
> >> > +                           *op++ = hex_asc_hi(ec >> 8);
> >> > +                           *op++ = hex_asc_lo(ec >> 8);
> >> > +                           *op++ = hex_asc_hi(ec);
> >> > +                           *op++ = hex_asc_lo(ec);
> >> Why doesn't this use pack_hex_byte()?
> > or snprintf
> sprintf looks like overkill here.

It's shorter and more intelligible though

	op += sprintf(op, ":%04x:%04x", etc) 

cheers, Joe