Closed
Bug 242183
Opened 20 years ago
Closed 20 years ago
CreateNewStreamConv and NS_NewStreamConv are silly/leaky/...
Categories
(Core :: Networking, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: timeless, Assigned: timeless)
References
()
Details
Attachments
(1 file)
2.06 KB,
patch
|
darin.moz
:
review+
darin.moz
:
superreview+
|
Details | Diff | Splinter Review |
72 static NS_IMETHODIMP 73 CreateNewStreamConv(nsISupports* aOuter, REFNSIID aIID, void **aResult) 74 { 75 if (!aResult) { 76 return NS_ERROR_INVALID_POINTER; 77 } 78 if (aOuter) { 79 *aResult = nsnull; 80 return NS_ERROR_NO_AGGREGATION; 81 } 82 nsStreamConverterService* inst = nsnull; 83 nsresult rv = NS_NewStreamConv(&inst); this block leaks because of the very messed up ownership (or lack thereof in NS_NewStreamConv): 84 if (NS_FAILED(rv)) { 85 *aResult = nsnull; 86 return rv; 87 } /block 88 rv = inst->QueryInterface(aIID, aResult); I don't see a reason for this block: 89 if (NS_FAILED(rv)) { 90 *aResult = nsnull; 91 } /block 92 NS_RELEASE(inst); /* get rid of extra refcnt */ 93 return rv; 94 } 690 NS_NewStreamConv(nsStreamConverterService** aStreamConv) 691 { 692 NS_PRECONDITION(aStreamConv != nsnull, "null ptr"); 693 if (!aStreamConv) return NS_ERROR_NULL_POINTER; 694 695 *aStreamConv = new nsStreamConverterService(); 696 if (!*aStreamConv) return NS_ERROR_OUT_OF_MEMORY; 697 698 NS_ADDREF(*aStreamConv); This returns failure for some OOM case: 699 return (*aStreamConv)->Init(); But what it should do is hold rv, and release *aStreamConv on failure. 700 }
Comment 1•20 years ago
|
||
> 83 nsresult rv = NS_NewStreamConv(&inst); >this block leaks because of the very messed up ownership (or lack thereof in >NS_NewStreamConv): that is NS_NewStreamConv's bug, not the one of this function. >I don't see a reason for this block: > 89 if (NS_FAILED(rv)) { > 90 *aResult = nsnull; > 91 } To make sure that the caller gets a null result when the operation failed?
the second bit is silly because QI will out null by contract
Attachment #147473 -
Flags: superreview?(darin)
Attachment #147473 -
Flags: review?(darin)
Updated•20 years ago
|
Attachment #147473 -
Flags: superreview?(darin)
Attachment #147473 -
Flags: superreview+
Attachment #147473 -
Flags: review?(darin)
Attachment #147473 -
Flags: review+
mozilla/netwerk/streamconv/src/nsStreamConvServiceFactory.cpp 1.14 mozilla/netwerk/streamconv/src/nsStreamConverterService.cpp 1.63
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•