Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 16 Sep 2015 23:25:56 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: ldr_split_line() performance regression

On Wed, Sep 16, 2015 at 03:11:57PM -0500, jfoug wrote:
> On 9/16/2015 3:03 PM, Solar Designer wrote:
> >         if (((*login)[0] == '+' && (!(*login)[1] || (*login)[1] == '@')) &&
> >             (*ciphertext)[0] != '$' &&
> >             strlen(*ciphertext) < 10 && strncmp(*ciphertext, "$dummy$", 7)) {
> 
> I would still recommend reversing the strlen and strncmp:
> 
>         if (((*login)[0] == '+' && (!(*login)[1] || (*login)[1] == '@')) &&
>             (*ciphertext)[0] != '$' && (*ciphertext)[1] != 'd' &&
>             strncmp(&((*ciphertext)[2]), "ummy$", 5) && strlen(*ciphertext) 
>             < 10) {
> 
> That code will almost never do the strncmp or strlen, UNLESS it is a dummy.

Not quite.  The strlen() is needed specifically for non-dummies, so it
will be reached most of the time, as long as *login passed the checks.

What's important is that *login won't pass those checks most of the time.
I think my version was good enough for that reason.

> >We could also use strnlen() there, if we depend on it elsewhere in the
> >jumbo tree anyway.
> 
> Fully agree to that.  If it must be used, then do it early, and use the 
> results.

OK, I'll let you and magnum sort it out.

In core, I won't currently commit a version with strnlen(), though.

Alexander

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.