515870 - GCC compiler warnings in NSS 3.12.4

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Wan-Teh Chang]

The patches in this bug fix the GCC compiler warnings in NSS 3.12.4.

Comment 1

[Wan-Teh Chang]

Attachment: Patch for jarver.c (checked in)

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.

Comment 2

[Wan-Teh Chang]

Attachment: Removed unused variable 'needCarry' (checked in)

Comment 3

[Wan-Teh Chang]

Attachment: Discard return value using (void) (checked in)

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.

Comment 4

[Wan-Teh Chang]

Attachment: Make conditional expressions explicit when they involve assignments (checked in)

Comment 5

[Wan-Teh Chang]

Attachment: unsigned char * vs. char * (checked in)

Comment 6

[Wan-Teh Chang]

Attachment: Use the offsetof macro defined in <stddef.h> (for SOFTOKEN_3_13_BRANCH) (checked in)

The next attachment is a test program that verifies the
two expressions evaluate to the same value.

Comment 7

[Wan-Teh Chang]

Attachment: Test program for offsetof

(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

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Is this really worthy of our time right now?

Comment 9

[Wan-Teh Chang]

That's why I didn't cc anyone on this bug, and why I only asked
you to review one trivial patch.

Comment 10

[Robert Relyea]

Comment on attachment 399953 [details] [diff] [review]
Removed unused variable 'needCarry' (checked in)

r+ rrelyea

Comment 11

[Robert Relyea]

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

Comment 12

[Wan-Teh Chang]

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

Comment 13

[Wan-Teh Chang]

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

Comment 14

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 399956 [details] [diff] [review]
Make conditional expressions explicit when they involve assignments (checked in)

r=nelson

Comment 15

[Nelson Bolyard (seldom reads bugmail)]

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 16

[Wan-Teh Chang]

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

Comment 17

[Wan-Teh Chang]

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 18

[Wan-Teh Chang]

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

Comment 19

[Wan-Teh Chang]

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