The message length should be serialized as a 64-bit value.

The message length should be serialized as a 64-bit value.

Also optimize the swapends function.

R=mark@chromium.org,rsleevi@chromium.org,yukawa@chromium.org
BUG=348333
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=282946

[wtc, 2014-07-12 01:23:40]

https://codereview.chromium.org/382423003/diff/1/base/sha1_portable.cc
File base/sha1_portable.cc (right):

https://codereview.chromium.org/382423003/diff/1/base/sha1_portable.cc#newcode62
base/sha1_portable.cc:62: uint32 cursor;

Yukawa also suggested that |cursor| should be defined as size_t. I didn't make
that change because |cursor| is the index for the |M| array (see line 59) and
only varies in the range [0...64].

https://codereview.chromium.org/382423003/diff/1/base/sha1_portable.cc#newcod...
base/sha1_portable.cc:154: M[cursor++] = l & 0xff;

All the & 0xff operations can be omitted.

[Ryan Sleevi, 2014-07-12 02:03:50]

lgtm

[yukawa, 2014-07-12 03:40:01]

LGTM.

https://codereview.chromium.org/382423003/diff/1/base/sha1_portable.cc
File base/sha1_portable.cc (right):

https://codereview.chromium.org/382423003/diff/1/base/sha1_portable.cc#newcode62
base/sha1_portable.cc:62: uint32 cursor;
On 2014/07/12 01:23:40, wtc wrote:
> 
> Yukawa also suggested that |cursor| should be defined as size_t. I didn't make
> that change because |cursor| is the index for the |M| array (see line 59) and
> only varies in the range [0...64].

Makes sense. Thanks.

[wtc, 2014-07-12 04:22:58]

The CQ bit was checked by wtc@chromium.org

[commit-bot: I haz the power, 2014-07-12 04:24:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wtc@chromium.org/382423003/1

[commit-bot: I haz the power, 2014-07-12 06:12:36]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-07-12 06:14:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-12 06:14:49]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[wtc, 2014-07-12 14:01:19]

Mark: please review this CL. Thanks.

The 'l' member (message length in bits) needs to be
serialized in big-endian order (most significant byte
first) as a 64-bit value. So eight bytes need to be
written. The current code assumes the high 32 bits are
all zeros and just writes four zero bytes (as part of
the while loop).

[Mark Mentovai, 2014-07-12 14:48:48]

LGTM

https://codereview.chromium.org/382423003/diff/1/base/sha1_portable.cc
File base/sha1_portable.cc (right):

https://codereview.chromium.org/382423003/diff/1/base/sha1_portable.cc#newcod...
base/sha1_portable.cc:154: M[cursor++] = l & 0xff;
wtc wrote:
> All the & 0xff operations can be omitted.

LGTM with or without this change.

[wtc, 2014-07-14 14:43:03]

The CQ bit was checked by wtc@chromium.org

[commit-bot: I haz the power, 2014-07-14 14:43:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wtc@chromium.org/382423003/1

[commit-bot: I haz the power, 2014-07-14 14:46:14]

Change committed as 282946