uninitialized variables in strsclnt.c

RESOLVED FIXED in 3.11.1

Status

NSS
Tools
P2
normal
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: wolfiR, Assigned: Julien Pierre)

Tracking

3.11
3.11.1
x86
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

12 years ago
strsclnt.c: In function 'launch_thread':
strsclnt.c:500: warning: 'i' is used uninitialized in this function
strsclnt.c: In function 'do_connects':
strsclnt.c:846: warning: 'now' is used uninitialized in this function

The first one is just used for a printf and not critical. I'm not sure for the second:
PRTime now;
PR_Lock(threadLock);
lastConnectSuccess = PR_MAX(now, lastConnectSuccess);

This seems broken to me at first glance.

Comment 1

12 years ago
Wolfgang, thanks for the bug report.

These bugs were introduced by the fix for bug 292151
in NSS 3.11.  The fix should be:
1. remove 'i'.  It probably should be replaced by 'numUsed-1'
   or 'numUsed' in the PRINTF statement.  The message
   "strsclnt: Launched thread in slot %d \n" may need to be
   updated to describe the new code (if the new code no longer
   uses slots).
2. initialize 'now' with PR_Now().
Assignee: wtchang → julien.pierre.bugs
Priority: -- → P1
Target Milestone: --- → 3.11.1
(Assignee)

Comment 2

12 years ago
Created attachment 209037 [details] [diff] [review]
fix as checked in to the tip

These are the fixes suggested by Wan-Teh, and they are correct.
Checking in strsclnt.c;
/cvsroot/mozilla/security/nss/cmd/strsclnt/strsclnt.c,v  <--  strsclnt.c
new revision: 1.45; previous revision: 1.44
done
(Assignee)

Updated

12 years ago
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 3

12 years ago
Changing priority. This is P2, not P1 . Target is 3.12 .
Status: RESOLVED → VERIFIED
Priority: P1 → P2
Target Milestone: 3.11.1 → 3.12

Comment 4

12 years ago
Julien, strsclnt is used in our test suite to determine
whether a build passes QA.  Also, we usually fix all the
compiler warnings in the libraries and the important
commands, but we didn't do that before the 3.11 release.
So your fix should be checked into the NSS_3_11_BRANCH
for 3.11.1.

Comment 5

12 years ago
I meant to say that
  we usually fix all the compiler warnings in the
  libraries and the important commands *before each
  minor release*, ...
(Assignee)

Comment 6

12 years ago
OK, I checked it in on the NSS_3_11_BRANCH :

Checking in strsclnt.c;
/cvsroot/mozilla/security/nss/cmd/strsclnt/strsclnt.c,v  <--  strsclnt.c
new revision: 1.43.2.1; previous revision: 1.43
done
Target Milestone: 3.12 → 3.11.1

Comment 7

12 years ago
Comment on attachment 209037 [details] [diff] [review]
fix as checked in to the tip

Julien,

I found that numUsed should only be used while holding
threadLock, so I suggest a different fix for "i".

We should keep "i".  Do not initialize it.

Set "i" here:

+     i = numUsed;
      slot = &threads[numUsed++];

And continue to use "i" in the PRINTF statement, which is
executed after we unlock threadLock.

      slot->inUse   = 1;
      PR_Unlock(threadLock);
      PRINTF("strsclnt: Launched thread in slot %d \n", i);
                                                       ^^^
Attachment #209037 - Flags: review-

Comment 8

12 years ago
Created attachment 209044 [details] [diff] [review]
Patch v2
Attachment #209037 - Attachment is obsolete: true
Attachment #209044 - Flags: review?(julien.pierre.bugs)

Comment 9

12 years ago
Created attachment 209045 [details] [diff] [review]
Patch v2 (corrected)
Attachment #209044 - Attachment is obsolete: true
Attachment #209045 - Flags: review?(julien.pierre.bugs)
Attachment #209044 - Flags: review?(julien.pierre.bugs)
reopened in light of most recent comments.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 11

12 years ago
Comment on attachment 209045 [details] [diff] [review]
Patch v2 (corrected)

Indeed, thanks for catching this. I have checked the corrections to both the tip and the 3.11 branch.

Checking in strsclnt.c;
/cvsroot/mozilla/security/nss/cmd/strsclnt/strsclnt.c,v  <--  strsclnt.c
new revision: 1.46; previous revision: 1.45


Checking in strsclnt.c;
/cvsroot/mozilla/security/nss/cmd/strsclnt/strsclnt.c,v  <--  strsclnt.c
new revision: 1.43.2.2; previous revision: 1.43.2.1
done
Attachment #209045 - Flags: review?(julien.pierre.bugs) → review+
(Assignee)

Updated

12 years ago
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.