|
Message-ID: <CAKGDhHUmfgJCbwwqQsT7COfntMGQnk-jnGKHuxBKOZORz9mD7g@mail.gmail.com> Date: Sun, 10 May 2015 19:38:04 +0200 From: Agnieszka Bielec <bielecagnieszka8@...il.com> To: john-dev@...ts.openwall.com Subject: Re: [john dev] [PHC support] parallel I repaired all things you noticed 2015-05-09 1:53 GMT+02:00 Solar Designer <solar@...nwall.com>: > 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.