Update SHA1_LENGTH -> kSHA1Length to match previous change to SHA256_LENGTH.

Update SHA1_LENGTH -> kSHA1Length to match previous change to SHA256_LENGTH.

(I didn't try and understand or fix why kSHA1Length is in base:: while kSHA256Length is in crypto::.)

BUG=92247
TEST=compiles
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=103179

[Peter Kasting, 2011-09-23 20:00:29]

By request...

[wtc, 2011-09-24 01:39:41]

LGTM on Patch Set 2.  Thanks!

Please make the following changes before you check this in.
I noticed that you discovered the compilation errors and
have uploaded Patch Set 4.  But those two files should not
be changed at all.

http://codereview.chromium.org/7972024/diff/8001/base/sha1.h
File base/sha1.h (right):

http://codereview.chromium.org/7972024/diff/8001/base/sha1.h#newcode17
base/sha1.h:17: const size_t kSHA1Length = 20;  // Length in bytes of a SHA-1
hash.
I asked you this question before but don't remember if you
answered it.  Could you tell me again whether we should use
'static' on a constant defined in a header file?  Thanks.

http://codereview.chromium.org/7972024/diff/8001/chrome/common/net/x509_certi...
File chrome/common/net/x509_certificate_model_nss.cc (right):

http://codereview.chromium.org/7972024/diff/8001/chrome/common/net/x509_certi...
chrome/common/net/x509_certificate_model_nss.cc:20: #include "crypto/sha2.h"
BUG: Please undo all the changes in this file.  This is one
reason I like the renaming.  It turns out that NSS has
macros named SHA1_LENGTH and SHA256_LENGTH:
http://mxr.mozilla.org/security/ident?i=SHA1_LENGTH
http://mxr.mozilla.org/security/ident?i=SHA256_LENGTH

So the references to SHA1_LENGTH and SHA256_LENGTH in
this file are actually to the NSS macros.  This is why
they don't have the base:: and crypto:: namespace prefixes.

http://codereview.chromium.org/7972024/diff/8001/crypto/hmac_win.cc
File crypto/hmac_win.cc (right):

http://codereview.chromium.org/7972024/diff/8001/crypto/hmac_win.cc#newcode15
crypto/hmac_win.cc:15: #include "crypto/sha2.h"
BUG(kind of): Please also undo the changes in this file.
The references to SHA256_LENGTH in this file are actually
to the SHA256_LENGTH macro defined in "crypto/third_party/nss/blapi.h",
which is an NSS file that we copied.

http://codereview.chromium.org/7972024/diff/8001/crypto/openpgp_symmetric_enc...
File crypto/openpgp_symmetric_encryption.cc (right):

http://codereview.chromium.org/7972024/diff/8001/crypto/openpgp_symmetric_enc...
crypto/openpgp_symmetric_encryption.cc:16: #include "base/sha1.h"
BUG: please undo the changes in this file.  This file uses
NSS even though that's not obvious from the file name.

http://codereview.chromium.org/7972024/diff/8001/net/base/transport_security_...
File net/base/transport_security_state.cc (left):

http://codereview.chromium.org/7972024/diff/8001/net/base/transport_security_...
net/base/transport_security_state.cc:17: #endif
Excellent!

http://codereview.chromium.org/7972024/diff/8001/net/base/x509_certificate_un...
File net/base/x509_certificate_unittest.cc (right):

http://codereview.chromium.org/7972024/diff/8001/net/base/x509_certificate_un...
net/base/x509_certificate_unittest.cc:673:
HexEncode(verify_result.public_key_hashes[1].data, base::kSHA1Length));
Nit: I like the original indentation better.  Will the line
be too long if you don't adjust the indentation?

http://codereview.chromium.org/7972024/diff/8001/net/socket/dns_cert_provenan...
File net/socket/dns_cert_provenance_checker.cc (right):

http://codereview.chromium.org/7972024/diff/8001/net/socket/dns_cert_provenan...
net/socket/dns_cert_provenance_checker.cc:27: #include "crypto/sha2.h"
Please undo the changes in this file.  This file uses NSS.

[Peter Kasting, 2011-09-25 20:52:01]

http://codereview.chromium.org/7972024/diff/8001/base/sha1.h
File base/sha1.h (right):

http://codereview.chromium.org/7972024/diff/8001/base/sha1.h#newcode17
base/sha1.h:17: const size_t kSHA1Length = 20;  // Length in bytes of a SHA-1
hash.
On 2011/09/24 01:39:41, wtc wrote:
> I asked you this question before but don't remember if you
> answered it.  Could you tell me again whether we should use
> 'static' on a constant defined in a header file?  Thanks.

This turns out to be an interesting C-vs.C++-ism.  In C, file-scope variables
with no explicit "static" or "extern" declaration default to extern linkage.  In
C++, they default to static linkage if they are "const" and extern linkage
otherwise.  So in C++, writing "static" on this const variable does not make a
difference, but in C it does (and would be necessary, since extern linkage here
would lead to multiply-defined symbols).

I have no particular feeling as to whether we should explicitly say "static"
here for clarity.  I removed it here and on kSHA256Length because I thought you
were complaining about it in the other case :)

http://codereview.chromium.org/7972024/diff/8001/chrome/common/net/x509_certi...
File chrome/common/net/x509_certificate_model_nss.cc (right):

http://codereview.chromium.org/7972024/diff/8001/chrome/common/net/x509_certi...
chrome/common/net/x509_certificate_model_nss.cc:20: #include "crypto/sha2.h"
On 2011/09/24 01:39:41, wtc wrote:
> So the references to SHA1_LENGTH and SHA256_LENGTH in
> this file are actually to the NSS macros.  This is why
> they don't have the base:: and crypto:: namespace prefixes.

Right.  I meant to be clearer about this in my change description -- I was
intentionally changing all non-third_party code to use the chrome constants, and
leaving third_party/*nss* using the NSS macros, in hopes of consistency and
fewer (possibly implicit) dependencies on things defined in the NSS headers.

I can change them back, but do you have a feeling about the above idea, or any
rules on when or how we should pick one over the other?  If we're not drawing
the line at the boundary of third_party, things seem a lot more subtle.

http://codereview.chromium.org/7972024/diff/8001/net/base/x509_certificate_un...
File net/base/x509_certificate_unittest.cc (right):

http://codereview.chromium.org/7972024/diff/8001/net/base/x509_certificate_un...
net/base/x509_certificate_unittest.cc:673:
HexEncode(verify_result.public_key_hashes[1].data, base::kSHA1Length));
On 2011/09/24 01:39:41, wtc wrote:
> Nit: I like the original indentation better.  Will the line
> be too long if you don't adjust the indentation?

Yes.

[wtc, 2011-09-26 18:06:22]

http://codereview.chromium.org/7972024/diff/8001/base/sha1.h
File base/sha1.h (right):

http://codereview.chromium.org/7972024/diff/8001/base/sha1.h#newcode17
base/sha1.h:17: const size_t kSHA1Length = 20;  // Length in bytes of a SHA-1
hash.
On 2011/09/25 20:52:01, Peter Kasting wrote:
> 
> I have no particular feeling as to whether we should explicitly say "static"
> here for clarity.  I removed it here and on kSHA256Length because I thought
you
> were complaining about it in the other case :)

Thank you for the explanation.  I didn't know exactly how
this works in C++, so I may have complained about it in the
other CL.  Sorry about that.

Please do what you think is right for C++.

http://codereview.chromium.org/7972024/diff/8001/chrome/common/net/x509_certi...
File chrome/common/net/x509_certificate_model_nss.cc (right):

http://codereview.chromium.org/7972024/diff/8001/chrome/common/net/x509_certi...
chrome/common/net/x509_certificate_model_nss.cc:20: #include "crypto/sha2.h"
On 2011/09/25 20:52:01, Peter Kasting wrote:
>
> I can change them back, but do you have a feeling about the above idea, or any
> rules on when or how we should pick one over the other?

The rule is that a file that uses the NSS hash functions
should also use the NSS hash-length macros.  There are
such files outside third_party as well.  Usually such
files are named xxx_nss.cc.

[Peter Kasting, 2011-09-26 20:08:18]

Waiting to upload until Roger lands the RLZ side and I can update DEPS.

http://codereview.chromium.org/7972024/diff/8001/base/sha1.h
File base/sha1.h (right):

http://codereview.chromium.org/7972024/diff/8001/base/sha1.h#newcode17
base/sha1.h:17: const size_t kSHA1Length = 20;  // Length in bytes of a SHA-1
hash.
On 2011/09/26 18:06:22, wtc wrote:
> Please do what you think is right for C++.

OK.  Explicitly saying "static" is probably clearer, so I've done that.

http://codereview.chromium.org/7972024/diff/8001/chrome/common/net/x509_certi...
File chrome/common/net/x509_certificate_model_nss.cc (right):

http://codereview.chromium.org/7972024/diff/8001/chrome/common/net/x509_certi...
chrome/common/net/x509_certificate_model_nss.cc:20: #include "crypto/sha2.h"
On 2011/09/26 18:06:22, wtc wrote:
> The rule is that a file that uses the NSS hash functions
> should also use the NSS hash-length macros.

OK.  I reverted the four files you mentioned needing reverts.

[Peter Kasting, 2011-09-28 20:43:47]

Brett, can you OWNERS r+?

[brettw, 2011-09-28 21:11:44]

LGTM rubberstamp