FMM in nsMsgNewsFolder::GetNewsrcLine(char * *) [nsNewsFolder.cpp:1545]

VERIFIED FIXED

Status

MailNews Core
Backend
VERIFIED FIXED
18 years ago
10 years ago

People

(Reporter: Daniel Bratell, Assigned: (not reading, please use seth@sspitzer.org instead))

Tracking

Trunk
x86
Windows 2000

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

18 years ago
There is a Free Memory Mismatch in nsMsgNewsFolder::GetNewsrcLine(char * *) 
[nsNewsFolder.cpp:1545]

setStr is allocated with PR_Malloc in nsNewsDatabase::GetReadSetStr(...) called 
at line 1541 in nsNewsFolder.cpp. It is then deallocated with delete[] at line 
1545. Since it was allocated with PR_Malloc, it should be deallocated with 
PR_Free.

From Purify help:
Sometimes mismatched frees work without crashing. However, you should watch out 
for changes in the implementation of any of the allocation modules (for example, 
between compilers or versions of operating systems) that might cause problems.
QA Contact: esther → stephend
news -> me.

taking from mscott.

thanks for catching is FMM.
Status: NEW → ASSIGNED
fix in hand, with some extra code cleanup.

daniel, can you review?
Assignee: mscott → sspitzer
Status: ASSIGNED → NEW
Created attachment 23794 [details] [diff] [review]
fix for the FMM, and other stuff.
(Reporter)

Comment 4

18 years ago
+    if (!*setStr) return NS_ERROR_FAILURE;
+    if (!setStr[0]) return NS_ERROR_FAILURE;

These two lines are doing the same thing. I think you meant to just check the
setStr pointer on the first line. With that change r=bratell.

Status: NEW → ASSIGNED
thanks for catching the problem.

actually, the first line is correct, the second line is wrong.
setStr is a char **, so the first one checks if the char * is null
and the second should (but didn't) check that the char * isn't just the '\0';

It should be:

+    if (!*setStr) return NS_ERROR_FAILURE;
+    if (!*setStr[0]) return NS_ERROR_FAILURE;

I'll attach a new patch.
Created attachment 23828 [details] [diff] [review]
new patch, please review.
(Reporter)

Comment 7

18 years ago
A couple of tabs disturbs the indentation.

Besides that I don't see anything strange. r=bratell

Comment 8

18 years ago
sr=bienvenu
fixed.  to verify, we'll need to use purify.

daniel, can you verify?
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
(Reporter)

Comment 10

18 years ago
I ran news for a while under Purify but didn't see the FMM. Marking fixed.
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.