The default bug view has changed. See this FAQ.

item cleanup is unused DEADCODE in SECITEM_AllocItem loser

RESOLVED FIXED in 3.12

Status

NSS
Libraries
P4
trivial
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: timeless, Assigned: Wan-Teh Chang)

Tracking

({coverity})

3.11
3.12
All
Linux
coverity

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: CID 300, URL)

(Reporter)

Description

11 years ago
found by coverity
I'm not sure, but I think this bug suggests that SECITEM_AllocItem is
unused in mozilla and therefore should be removed from NSS.  

The mozilla family of client products are not the only users of NSS. 
The mere fact that mozilla doesn't use a feature doesn't mean the feature
should be removed.  

Updated

11 years ago
Assignee: kengert → nobody
Hmm.  On re-review, I guess it is asserting that this statement
 91   	if (item != NULL) {
after the label "loser" can never be true.
I think in the past, SECITEM_AllocItem would "goto loser"
if the allocation for item->data failed, which is how the
above "if" expression could have been true.  

The question in my mind is: shold SECITEM_AllocItem reutrn
a NULL "item", or an item with a null "data" pointer when 
it cannot allocate the data.  

Either way, we won't worry about this until well after NSS 3.11.1
is released.
Priority: -- → P4
Hardware: PC → All
Target Milestone: --- → 3.11.1
Version: unspecified → 3.11
(Assignee)

Comment 3

11 years ago
Nelson, you asked:
> The question in my mind is: shold SECITEM_AllocItem reutrn
> a NULL "item", or an item with a null "data" pointer when 
> it cannot allocate the data.

This issue is the subject of bug 320336.
(Assignee)

Comment 4

11 years ago
My patch for bug 320336 fixes this bug as a by-product
because it makes the dead code reachable now.  I've
checked in my patch on the NSS trunk (3.12).  So this
bug is fixed in NSS 3.12.

Nelson, if you really want this bug (severity=trivial)
fixed in NSS 3.11.1, do you want me to fix it by removing
the dead code or with my patch for bug 320336?
Assignee: nobody → wtchang
Wan-Teh, I agree that *this* bug is trivial. 
But I think your fix for bug 320336 should be on both trunk and branch.
And since it fixes this bug as a by-product, that's even better.
Depends on: 320336
(Assignee)

Comment 6

11 years ago
OK, this bug is fixed in NSS 3.11.1 (by making the
dead code reachable).  The patch is in bug 320336.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 7

11 years ago
The patch was backed out because it changed the function's
behavior when the 'len' argument is 0.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 8

11 years ago
Moved to NSS 3.11.2 because bug 320336, which this bug depends on,
was moved to NSS 3.11.2.
Status: REOPENED → ASSIGNED
Target Milestone: 3.11.1 → 3.11.2
(Assignee)

Comment 9

11 years ago
I only checked in the patch (in bug 320336) on the NSS trunk (3.12),
partly because Nelson is worried about unintended consequences, and
partly because lib/util is used by lib/freebl, and we want to finish
our FIPS algorithm testing this week.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
Target Milestone: 3.11.2 → 3.12
CID 300
Whiteboard: CID 300
You need to log in before you can comment on or make changes to this bug.