Closed Bug 515870 Opened 15 years ago Closed 14 years ago

GCC compiler warnings in NSS 3.12.4

Categories

(NSS :: Libraries, defect)

defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED FIXED
3.12.6

People

(Reporter: wtc, Assigned: wtc)

References

Details

Attachments

(7 files)

The patches in this bug fix the GCC compiler warnings in NSS 3.12.4.
x_name is the name of an array:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/jar/jarver.c&rev=1.17&mark=325#325

therefore x_name cannot be a null pointer.  So
!x_name is always false.
Attachment #399953 - Flags: review?(rrelyea)
When the second argument to nssTrustDomain_GetCertsFromCache
is not NULL, nssTrustDomain_GetCertsFromCache returns NULL:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/pki/tdcache.c&rev=1.48&mark=1142,1145,1158,1160,1165#1139

Use (void) instead of (void *) to discard the useless NULL
return value.
The next attachment is a test program that verifies the
two expressions evaluate to the same value.
Attachment #399958 - Flags: review?(rrelyea)
(You can also count the struct members to see 'names' is at offset 32.)

Sample output of the test program when compiled for 32-bit and
64-bit Linux:

wtc@aes:/usr/local/google/home/wtc/nss-tip$ gcc -m32 offsetof.c
offsetof.c: In function 'main':
offsetof.c:28: warning: cast from pointer to integer of different size
wtc@aes:/usr/local/google/home/wtc/nss-tip$ ./a.out
offset1=32
offset2=32
wtc@aes:/usr/local/google/home/wtc/nss-tip$ gcc -m64 offsetof.c 
offsetof.c: In function 'main':
offsetof.c:28: warning: cast from pointer to integer of different size
wtc@aes:/usr/local/google/home/wtc/nss-tip$ ./a.out
offset1=32
offset2=32
Is this really worthy of our time right now?
That's why I didn't cc anyone on this bug, and why I only asked
you to review one trivial patch.
Comment on attachment 399953 [details] [diff] [review]
Removed unused variable 'needCarry' (checked in)

r+ rrelyea
Attachment #399953 - Flags: review?(rrelyea) → review+
Comment on attachment 399958 [details] [diff] [review]
Use the offsetof macro defined in <stddef.h> (for SOFTOKEN_3_13_BRANCH) (checked in)

r+ for the SOFTOKEN_3_13 branch

bob
Attachment #399958 - Flags: review?(rrelyea) → review+
Comment on attachment 399953 [details] [diff] [review]
Removed unused variable 'needCarry' (checked in)

Checked in on the NSS trunk (NSS 3.12.5).

Checking in pk11merge.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11merge.c,v  <--  pk11merge.c
new revision: 1.6; previous revision: 1.5
done
Attachment #399953 - Attachment description: Removed unused variable 'needCarry' → Removed unused variable 'needCarry' (checked in)
Comment on attachment 399958 [details] [diff] [review]
Use the offsetof macro defined in <stddef.h> (for SOFTOKEN_3_13_BRANCH) (checked in)

Checked in on the SOFTOKEN_3_13_BRANCH (Softoken 3.13).

Checking in pk11db.c;
/cvsroot/mozilla/security/nss/lib/softoken/legacydb/pk11db.c,v  <--  pk11db.c
new revision: 1.2.54.1; previous revision: 1.2
done
Attachment #399958 - Attachment description: Use the offsetof macro defined in <stddef.h> (for SOFTOKEN_3_13_BRANCH) → Use the offsetof macro defined in <stddef.h> (for SOFTOKEN_3_13_BRANCH) (checked in)
Comment on attachment 399956 [details] [diff] [review]
Make conditional expressions explicit when they involve assignments (checked in)

r=nelson
Attachment #399956 - Attachment description: Add parentheses around assignments in conditional expressions → Make conditional expressions explicit when they involve assignments
Attachment #399956 - Flags: review?(nelson) → review+
Comment on attachment 399955 [details] [diff] [review]
Discard return value using (void) (checked in)

Personally, I would prefer that you just eliminate the unnecessary casts altogether, and use some sort of command line option to disable the stupid warning.  
There are pedantic compilers that will whine about it either way.  I know of one compiler that will whine that the cast has no effect (DUH!).  
Death to stupid pedantry!
Comment on attachment 399951 [details] [diff] [review]
Patch for jarver.c (checked in)

I checked in this patch on the NSS trunk (NSS 3.12.6).

Checking in jarver.c;
/cvsroot/mozilla/security/nss/lib/jar/jarver.c,v  <--  jarver.c
new revision: 1.18; previous revision: 1.17
done
Attachment #399951 - Attachment description: Patch for jarver.c → Patch for jarver.c (checked in)
Attachment #399956 - Attachment description: Make conditional expressions explicit when they involve assignments → Make conditional expressions explicit when they involve assignments (checked in)
Comment on attachment 399956 [details] [diff] [review]
Make conditional expressions explicit when they involve assignments (checked in)

I checked in the patch on the NSS trunk (NSS 3.12.6).

Checking in lib/certdb/secname.c;
/cvsroot/mozilla/security/nss/lib/certdb/secname.c,v  <--  secname.c
new revision: 1.26; previous revision: 1.25
done
Checking in lib/libpkix/pkix/util/pkix_list.c;
/cvsroot/mozilla/security/nss/lib/libpkix/pkix/util/pkix_list.c,v  <--  pkix_list.c
new revision: 1.15; previous revision: 1.14
done
Comment on attachment 399957 [details] [diff] [review]
unsigned char * vs. char * (checked in)

I checked in this patch on the NSS trunk (NSS 3.12.6).

Checking in cmd/ssltap/ssltap.c;
/cvsroot/mozilla/security/nss/cmd/ssltap/ssltap.c,v  <--  ssltap.c
new revision: 1.18; previous revision: 1.17
done
Checking in lib/certdb/alg1485.c;
/cvsroot/mozilla/security/nss/lib/certdb/alg1485.c,v  <--  alg1485.c
new revision: 1.39; previous revision: 1.38
done
Checking in lib/certdb/certdb.c;
/cvsroot/mozilla/security/nss/lib/certdb/certdb.c,v  <--  certdb.c
new revision: 1.102; previous revision: 1.101
done
Checking in lib/pkcs7/certread.c;
/cvsroot/mozilla/security/nss/lib/pkcs7/certread.c,v  <--  certread.c
new revision: 1.17; previous revision: 1.16
done
Attachment #399957 - Attachment description: unsigned char * vs. char * → unsigned char * vs. char * (checked in)
Comment on attachment 399955 [details] [diff] [review]
Discard return value using (void) (checked in)

I checked in this patch on the NSS trunk (NSS 3.12.6) by mistake.
I thought Nelson gave his review+ in comment 15.

Elio, could you review this patch?  It's clear the intent of the (void *)
casts is to discard the function's return value.

Checking in lib/pk11wrap/pk11cert.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11cert.c,v  <--  pk11cert.c
new revision: 1.173; previous revision: 1.172
done
Checking in lib/pki/tdcache.c;
/cvsroot/mozilla/security/nss/lib/pki/tdcache.c,v  <--  tdcache.c
new revision: 1.49; previous revision: 1.48
done
Checking in lib/pki/trustdomain.c;
/cvsroot/mozilla/security/nss/lib/pki/trustdomain.c,v  <--  trustdomain.c
new revision: 1.61; previous revision: 1.60
done
Attachment #399955 - Attachment description: Discard return value using (void) → Discard return value using (void) (checked in)
Attachment #399955 - Flags: review?(emaldona)
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.12.6
Attachment #399955 - Flags: review?(emaldona) → review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: