Fixes to compiler warnings in MSVC

Fixes to compiler warnings in MSVC

[Bill Richardson, 2010-06-21 23:32:11]

LGTM

[gauravsh, 2010-06-21 23:32:48]

LGTM with comment

We should be consistent about the data type for 'algorithm' - and use the same
one everywhere. Although valid values of algorithm fit in uint8_t, I also see
uint32_t and uint64_t used at various places.

http://codereview.chromium.org/2851015/diff/2001/3005
File firmware/lib/cryptolib/rsa.c (right):

http://codereview.chromium.org/2851015/diff/2001/3005#newcode88
firmware/lib/cryptolib/rsa.c:88: for (i = 0; i < (int)key->len; ++i) {
both i and key->len are uint32_t, so this cast seems unncessary

http://codereview.chromium.org/2851015/diff/2001/3005#newcode111
firmware/lib/cryptolib/rsa.c:111: for (i = (int)key->len - 1; i >= 0; --i) {
i and key->len are both uint32_t

http://codereview.chromium.org/2851015/diff/2001/3011
File firmware/lib/vboot_common.c (right):

http://codereview.chromium.org/2851015/diff/2001/3011#newcode115
firmware/lib/vboot_common.c:115: if (RSAProcessedKeySize((int)key->algorithm) !=
(int)key->key_size) {
so what is the data type for algorithm? I saw uint8_t used for RSAVerify(), it
is uint64_t in VbPublicKey (presumably for alignment), and it is an int here. 

We should be consistent here to easily trace overflow errors as this gets passed
around.

[Randall Spangler, 2010-06-21 23:57:52]

On 2010/06/21 23:32:48, gauravsh wrote:
> LGTM with comment
> 
> We should be consistent about the data type for 'algorithm' - and use the same
> one everywhere. Although valid values of algorithm fit in uint8_t, I also see
> uint32_t and uint64_t used at various places.

Good point.  Will clean that up in a followup CL.

> 
> http://codereview.chromium.org/2851015/diff/2001/3005
> File firmware/lib/cryptolib/rsa.c (right):
> 
> http://codereview.chromium.org/2851015/diff/2001/3005#newcode88
> firmware/lib/cryptolib/rsa.c:88: for (i = 0; i < (int)key->len; ++i) {
> both i and key->len are uint32_t, so this cast seems unncessary
> 
> http://codereview.chromium.org/2851015/diff/2001/3005#newcode111
> firmware/lib/cryptolib/rsa.c:111: for (i = (int)key->len - 1; i >= 0; --i) {
> i and key->len are both uint32_t
> 
> http://codereview.chromium.org/2851015/diff/2001/3011
> File firmware/lib/vboot_common.c (right):
> 
> http://codereview.chromium.org/2851015/diff/2001/3011#newcode115
> firmware/lib/vboot_common.c:115: if (RSAProcessedKeySize((int)key->algorithm)
!=
> (int)key->key_size) {
> so what is the data type for algorithm? I saw uint8_t used for RSAVerify(), it
> is uint64_t in VbPublicKey (presumably for alignment), and it is an int here. 
> 
> We should be consistent here to easily trace overflow errors as this gets
passed
> around.

[Randall Spangler, 2010-06-21 23:58:05]

http://codereview.chromium.org/2851015/diff/2001/3005
File firmware/lib/cryptolib/rsa.c (right):

http://codereview.chromium.org/2851015/diff/2001/3005#newcode88
firmware/lib/cryptolib/rsa.c:88: for (i = 0; i < (int)key->len; ++i) {
On 2010/06/21 23:32:48, gauravsh wrote:
> both i and key->len are uint32_t, so this cast seems unncessary

i is a (signed) int; key->len is a uint32_t.  So MSVC gives a signed/unsigned
comparison warning.

[gauravsh, 2010-06-22 00:00:31]

On Mon, Jun 21, 2010 at 4:58 PM,  <rspangler@chromium.org> wrote:
>
> http://codereview.chromium.org/2851015/diff/2001/3005
> File firmware/lib/cryptolib/rsa.c (right):
>
> http://codereview.chromium.org/2851015/diff/2001/3005#newcode88
> firmware/lib/cryptolib/rsa.c:88: for (i = 0; i < (int)key->len; ++i) {
> On 2010/06/21 23:32:48, gauravsh wrote:
>>
>> both i and key->len are uint32_t, so this cast seems unncessary
>
> i is a (signed) int; key->len is a uint32_t.  So MSVC gives a
> signed/unsigned comparison warning.

Yeah, just realized that I was distracted by the bright and green
uint32_t i declared in the function above in the code review. :)
>
> http://codereview.chromium.org/2851015/show
>



-- 
-g