Last Comment Bug 333405 - item cleanup is unused DEADCODE in SECITEM_AllocItem loser
: item cleanup is unused DEADCODE in SECITEM_AllocItem loser
CID 300
: coverity
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.11
: All Linux
: P4 trivial (vote)
: 3.12
Assigned To: Wan-Teh Chang
Depends on: 320336
  Show dependency treegraph
Reported: 2006-04-09 23:44 PDT by timeless
Modified: 2006-06-10 18:45 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---


Description timeless 2006-04-09 23:44:55 PDT
found by coverity
Comment 1 Nelson Bolyard (seldom reads bugmail) 2006-04-10 00:22:15 PDT
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.  
Comment 2 Nelson Bolyard (seldom reads bugmail) 2006-04-17 16:42:04 PDT
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.
Comment 3 Wan-Teh Chang 2006-04-22 07:38:06 PDT
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.
Comment 4 Wan-Teh Chang 2006-04-24 15:57:24 PDT
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?
Comment 5 Nelson Bolyard (seldom reads bugmail) 2006-04-24 16:36:31 PDT
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.
Comment 6 Wan-Teh Chang 2006-04-24 16:52:43 PDT
OK, this bug is fixed in NSS 3.11.1 (by making the
dead code reachable).  The patch is in bug 320336.
Comment 7 Wan-Teh Chang 2006-04-25 09:39:08 PDT
The patch was backed out because it changed the function's
behavior when the 'len' argument is 0.
Comment 8 Wan-Teh Chang 2006-04-27 10:19:29 PDT
Moved to NSS 3.11.2 because bug 320336, which this bug depends on,
was moved to NSS 3.11.2.
Comment 9 Wan-Teh Chang 2006-05-22 15:53:13 PDT
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.
Comment 10 Nelson Bolyard (seldom reads bugmail) 2006-06-10 18:45:35 PDT
CID 300

Note You need to log in before you can comment on or make changes to this bug.