Closed
Bug 6926
Opened 25 years ago
Closed 25 years ago
MLK: leak in nsProfile::GetCurrentProfileDir()
Categories
(Core Graveyard :: Profile: BackEnd, defect, P3)
Core Graveyard
Profile: BackEnd
Tracking
(Not tracked)
VERIFIED
FIXED
M7
People
(Reporter: bruce, Assigned: racham)
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
This leads to the largest leak in M6 (over time).
nsGlobalHistory::GetHistoryDir() calls nsProfile::GetCurrentProfileDir().
GetCurrentProfileDir() calls GetCurrentProfile() with the address of a char*
profileName.
nsProfile::GetCurrentProfile() does:
*profileName = (char*) PR_Malloc(sizeof(char) * _MAX_LENGTH);
A couple of lines later, it does (when an operation has failed):
profileName = '\0';
This would appear to be the source of the leak, as back up one stack frame in
nsProfile::GetCurrentProfileDir(), it checks to see if profileName (a local var
in that scope) is null or not, and if not, it PR_DELETE()s it. this assignment
of profileName to '\0' while it is a char** would appear incorrect. If it is
correct, then the memory needs to be freed up somewhere.
For a related bug (and why this code is getting called so often), check out bug
#6922.
Comment 1•25 years ago
|
||
selmer, racham, gayatrib; is there anyone to look at this over the weekend?
Reporter | ||
Comment 2•25 years ago
|
||
Okay. Nothing so complicated as all that. Found the problem and going to attach
a patch in about 5 minutes. Don't know the profileName = '\0' thing is a
problem or not, but m_reg->GetString() allocates memory, so you don't need to
allocate memory before passing the char** into it.
Reporter | ||
Comment 3•25 years ago
|
||
Reporter | ||
Comment 4•25 years ago
|
||
Well, looking around a bit more, there's another instance of this same thing
happening elsewhere. Lines 933 and 934 of version 1.11 of nsProfile.cpp down in
nsProfile::CopyRegKey() have the same errors. These mallocs should not be
needed. A quick glance at all of the other uses of nsRegistry::GetString()
don't seem to show any other violations.
Updated•25 years ago
|
Target Milestone: M6
Comment 5•25 years ago
|
||
bruce, do you want to check the patch in for M6?
Reporter | ||
Comment 6•25 years ago
|
||
sure. will do so today.
Reporter | ||
Comment 7•25 years ago
|
||
Fix checked in for the main problem. Don't want to mark this fixed until the
whole file has been reviewed and fixed.
Reporter | ||
Comment 8•25 years ago
|
||
I nailed down the critical portion. Should we move the rest of the bug (the
review of the rest of the file) to M7?
Updated•25 years ago
|
Target Milestone: M6 → M7
Comment 9•25 years ago
|
||
ok. -> m7. how big is the current leak after your first fix?
Comment 10•25 years ago
|
||
It's really a pain to have a single bug track multiple problems/actions.
Bhuvan, please open a new bug for the review process and then mark this one
fixed. Steve
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 11•25 years ago
|
||
Bug #6984 to continue the review of the other segments of profile code.
Updated•25 years ago
|
Status: RESOLVED → VERIFIED
Comment 12•25 years ago
|
||
Moving all Profile Manager bugs to new Profile Manager Backend component.
Profile Manager component to be deleted.
Updated•9 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•