1011090 - CERTDB_* flags are signed but always used in unsigned context (signed/unsigned conversion warnings)

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: make-CERTDB_-unsigned.patch

This patch makes the CERTDB_* flags unsigned so that they will no longer trigger signed/unsigned conversion warnings in NSS or in applications.

Note that these flags are used for testing values of the members of CERTCertTrust, which are all unsigned:

    struct CERTCertTrustStr {
        unsigned int sslFlags;
        unsigned int emailFlags;
        unsigned int objectSigningFlags;
    };

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Thanks for the quick review!

http://hg.mozilla.org/projects/nss/rev/e6aec6970d0a

Comment 2

[Wan-Teh Chang]

Attachment: Remove a duplication definition of the CERTDB_USER macro in lib/jar/jarver.c

With the previous patch to certdb.h, Visual C++ now warns about
a duplicate definition of CERTDB_USER in lib/jar/jarver.c:

cl -FoWIN954.0_DBG.OBJ/jarver.obj -c -Zi -FdWIN954.0_DBG.OBJ/ -Od -W3 -nologo -D
_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -MD -we4002 -we4003 -we4004 -
we4006 -we4009 -we4013 -we4015 -we4028 -we4033 -we4035 -we4045 -we4047 -we4053 -
we4054 -we4063 -we4064 -we4078 -we4087 -we4090 -we4098 -we4390 -we4551 -we4553 -
we4715 -DXP_PC -DMOZILLA_CLIENT=1 -DDEBUG -D_DEBUG -UNDEBUG -DDEBUG_Wan_Teh_Chan
g -DWIN32 -D_X86_ -D_WINDOWS -DWIN95 -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT -D
SSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES -DNSS_X86_OR_X64 -DNSS_X86 -I../../../
dist/WIN954.0_DBG.OBJ/include -I../../../dist/public/nss -I../../../dist/private
/nss  "/d/nss-tip.8/nss/lib/jar/jarver.c"
jarver.c
d:/nss-tip.8/nss/lib/jar/jarver.c(22) : warning C4005: 'CERTDB_USER' : macro red
efinition
        d:\nss-tip.8\dist\public\nss\certdb.h(16) : see previous definition of '
CERTDB_USER'

I think it is best to just delete the duplicate definition because
jarver.c already includes certdb.h.

Note that lib/softoken/legacydb/pcertt.h also defines these
CERTDB_ macros:
http://mxr.mozilla.org/nss/ident?i=CERTDB_USER

Although I believe pcertt.h and certdb.h are never used
together, it'd be good to make the same changes to pcertt.h
for consistency.

Comment 3

[Wan-Teh Chang]

Attachment: Make CERTDB_* constants in pcertt.h unsigned for consistency with certdb.h

Comment 4

[Wan-Teh Chang]

Attachment: Fix signed/unsigned comparison warnings in lib/softoken/legacydb

None of these warnings are related to the CERTDB_ constants.

Comment 5

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 8425517 [details] [diff] [review]
Make CERTDB_* constants in pcertt.h unsigned for consistency with certdb.h

Review of attachment 8425517 [details] [diff] [review]:
-----------------------------------------------------------------

I didn't even think to check for duplicate definitions of these flags. Thanks for fixing this.

Comment 6

[Wan-Teh Chang]

Checked in the three patches:
https://hg.mozilla.org/projects/nss/rev/1ed35c824d29
https://hg.mozilla.org/projects/nss/rev/3264638234f1
https://hg.mozilla.org/projects/nss/rev/b48e80f3f44d