linux-kernel - Re: [PATCH] kstrtox: reuse functions from ctype.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <op.vtxmkrzr3l0zgt@mnazarewicz-glaptop>
Date:	Thu, 14 Apr 2011 16:30:17 +0200
From:	"Michal Nazarewicz" <mina86@...a86.com>
To:	"Alexey Dobriyan" <adobriyan@...il.com>
Cc:	"Andrew Morton" <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kstrtox: reuse functions from ctype.h

On Thu, 14 Apr 2011 16:06:34 +0200, Alexey Dobriyan <adobriyan@...il.com>  
wrote:

> On Thu, Apr 14, 2011 at 4:34 PM, Michal Nazarewicz <mina86@...a86.com>  
> wrote:
>> kstrto*() family of functions uses open coded test
>> for a hexadecimal digit and
>
> Yes, so?

So it's better to avoid open-coded code without a reason if there is
already function for what one tries to do.

>> own implementation of tolower() function.
>
> No!
> It's special cased for this very usage, because the rest of ASCII is
> of no concern.
> It doesn't claim tolower() semantics.

Either way, the question is do we care about this optimisation so much
as to use it and somehow duplicate code which already is in ctype.h.

I'm not saying we don't.  I'm asking whether we want to.

>> @@ -47,8 +42,8 @@ static int _kstrtoull(const char *s, unsigned int  
>> base, unsigned long long *res)
>>
>>                if ('0' <= *s && *s <= '9')
>>                        val = *s - '0';
>> -               else if ('a' <= _tolower(*s) && _tolower(*s) <= 'f')
>> -                       val = _tolower(*s) - 'a' + 10;
>> +               else if (isxdigit(*s))
>
> [0-9] are isxdigit() as well, so the code sort of logically duplicate.

Yes, so? ;)

I think isxdigit(*s) looks nicer than “'a' <= _tolower(*s) &&
_tolower(*s) <= 'f'”.

>> +                       val = tolower(*s) - 'a' + 10;

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michal "mina86" Nazarewicz    (o o)
ooo +-----<email/xmpp: mnazarewicz@...gle.com>-----ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/