118639 - fix leaks in nsFTPDirListingConv.cpp

Status: VERIFIED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[Pete Collins (MDG)]

fix leaks and clean up file some.


--pete

Comment 1

[Pete Collins (MDG)]

Attachment: patch to fix leaks

Comment 2

[Pete Collins (MDG)]

Please review.

Comment 3

[Darin Fisher]

Comment on attachment 68423 [details] [diff] [review]
patch to fix leaks

>Index: nsFTPDirListingConv.cpp

>         escName = nsEscape(aLine, url_Path);
>-        aEntry->mName = escName;
>+        aEntry->mName.Adopt(escName);
>         nsMemory::Free(escName);

crash!


>     escName = nsEscape(ptr+1, url_Path);
>-    aEntry->mName = escName;
>+    aEntry->mName.Adopt(escName);
>     nsMemory::Free(escName);

crash!


etc... you're asking a string class to adopt a pointer, but then you free it?!?
if you're going to Adopt then don't Free.  ie, the string class subsumes
ownership
of the string when you ask it to adopt a character pointer.

Comment 4

[Pete Collins (MDG)]

Attachment: removing calls to Free

duh, I should have looked at things closer. 

Sorry.

--pete

Comment 5

[Darin Fisher]

Comment on attachment 68459 [details] [diff] [review]
removing calls to Free

>Index: nsFTPDirListingConv.cpp

>-                thisEntry->mName = escName;
>+                thisEntry->mName.Adopt(escName);
> 			}
>-            nsMemory::Free(escName);

how about cleaning up the indentation before checking in?

with that, sr=darin

Comment 6

[Pete Collins (MDG)]

Attachment: expanded tabs in file and cleaned up some intentation

Comment 7

[Doug Turner (:dougt)]

Comment on attachment 68536 [details] [diff] [review]
expanded tabs in file and cleaned up some intentation

r=dougt, sr=carried over from darin.

Comment 8

[Pete Collins (MDG)]

checked in 

--pete

Comment 9

[Bradley Baetz (:bbaetz)]

Comment on attachment 68536 [details] [diff] [review]
expanded tabs in file and cleaned up some intentation


>@@ -1083,7 +1076,6 @@
>         char *escapedDate = nsEscape(buffer, url_Path);
> 
>         aString.Append(escapedDate);
>-        nsMemory::Free(escapedDate);
>         aString.Append(' ');
> 


Doesn't this leak? You're not .Adopt()ing that one....

Comment 10

[Pete Collins (MDG)]

Attachment: Damn, thanks for catching that

Comment 11

[Pete Collins (MDG)]

Attachment: uh, wrong one, ignore previous patch

Comment 12

[Bradley Baetz (:bbaetz)]

Comment on attachment 68721 [details] [diff] [review]
uh, wrong one,  ignore previous patch 

r=me

Comment 13

[Pete Collins (MDG)]

Reopening to check in this fix.

--pete

Comment 14

[Pete Collins (MDG)]

Darin, l inadvertantly removed a needed nsMemory::Free.

can you sr this?

Thanks

--pete

Comment 15

[Darin Fisher]

Comment on attachment 68721 [details] [diff] [review]
uh, wrong one,  ignore previous patch 

sr=darin

Comment 16

[Asa Dotzler [:asa]]

a=asa (on behalf of drivers) for checkin to 0.9.9

Comment 17

[Pete Collins (MDG)]

Checked in.

--pete

Comment 18

[benc]

1.68pete%alphanumerica.comFeb 22 16:41 b=118639, r=bbaetz, sr=darin, a=asa
inadvertantly removed a needed nsMemory::Free when cleaning up leaks in this file.

--pete