342750 - NativeUConvService::GetNativeConverter leaks on failure

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[timeless]

this actually happens :)

Comment 1

[Simon Montagu :smontagu]

Reassigning per blame

Comment 2

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 3

[Benjamin Smedberg]

Comment on attachment 227259 [details] [diff] [review]
patch v.1

Calling Init() without having called AddRef() is bad in general... it can easily lead to early-deleted objects. And while you're here, the multiple casts are silly.

Comment 4

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Patch v2

Patch v2

* Use |NS_ADDREF| immediately after we know |ucl| was successfully allocated a pointer (and only a single direct cast too a nsISupports pointer type).
* Check if |aResult->Init| failed, reset |aResult| back to |nsnull| and delete the |ucl| object.

Comment 5

[Benjamin Smedberg]

Comment on attachment 250977 [details] [diff] [review]
Patch v2

>Index: intl/uconv/native/nsNativeUConvService.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/intl/uconv/native/nsNativeUConvService.cpp,v
>retrieving revision 1.5
>diff -u -4 -p -r1.5 nsNativeUConvService.cpp
>--- intl/uconv/native/nsNativeUConvService.cpp	6 Jan 2005 19:08:15 -0000	1.5
>+++ intl/uconv/native/nsNativeUConvService.cpp	9 Jan 2007 20:56:24 -0000
>@@ -375,12 +375,14 @@ NativeUConvService::GetNativeConverter(c
>     IConvAdaptor* ucl = new IConvAdaptor();
>     if (!ucl)
>         return NS_ERROR_OUT_OF_MEMORY;

The simple way to do this is to change ucl to a 

nsRefPtr<IConvAdaptor>

Then you get the addref/release pair for free.

> 
>-    nsresult rv = ucl->Init(from, to);
>+    NS_ADDREF(*aResult = (nsISupports*)ucl);

You should not need any cast here.

Comment 6

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Patch v3

Patch v3

Updated to Benjamin's comments.

Comment 7

[Benjamin Smedberg]

Comment on attachment 251116 [details] [diff] [review]
Patch v3

IANASR

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 251116 [details] [diff] [review]
Patch v3

YSBASR

Comment 9

[Nickolay_Ponomarev]

mozilla/intl/uconv/native/nsNativeUConvService.cpp 1.6