Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 9 May 2015 02:53:02 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: [john dev] [PHC support] parallel

Agnieszka,

On Sat, May 09, 2015 at 01:25:22AM +0200, Agnieszka Bielec wrote:
> I've implemented the basic version of parallel (PHS) (SHA-512).

I took a look at commit 5746012c73e10f29d662c02f4421859d15589d91.

Here are some assorted issues I noticed, in arbitrary order:

You edited parallel.h compared to Steve's original, yet your edited file
still says it's by Steve, and does not mention you.  This is wrong.
Steve shouldn't be responsible for your bugs, if any, and it should be
clear who the copyright holders of each file are and what it's license
is.  Steve's parallel.h doesn't mention a license, but perhaps it was
elsewhere in the Parallel package?

In fact, it's unclear which Parallel .h file your parallel.h is based
on.  Can you clarify?

"#ifdef ARCH_LITTLE_ENDIAN" is wrong.  We use "#if ARCH_LITTLE_ENDIAN"
elsewhere, since we define this to either 0 or 1.

Among the test vectors, please include some with non-zero sequential
cost.  Notice how Parallel uses its t_cost:

        parallelLoops = 3 * 5 * 128 * calcLoopCount(t_cost & 0xffff);
        sequentialLoops = calcLoopCount(t_cost >> 16);

What do you mean by "//to do: change char to short integers" for
get_salt()?

cmp_all() and cmp_one() include some magic for length 256, I guess
inherited from POMELO and irrelevant for Parallel.

The "struct fmt_main fmt_parallel" initializer is badly mis-formatted,
just like POMELO's.  You need to format them properly.  Please see
dummy.c for a cleaner format definition.

Overall, this stuff looks extremely dirty.  You need to clean it up
before you proceed to work on it further.

After the cleanups, your next priority may be OpenCL implementation, and
after that (or while waiting for feedback from me) a SIMD implementation.

Thanks,

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.