13289 - MLK: char* leaked in nsProfile::GetProfileDir()

Status: RESOLVED FIXED

(Core Graveyard :: Profile: BackEnd, defect, P3)

Description

[Bruce Mitchener]

This is really in the backend in nsProfile.cpp
In nsProfile::GetProfileDir(), it does this:
orgProfileDir = PL_strdup(profileDir->GetCString());  (speaking of which, this
should be using nsCRT::strdup() instead of PL_strdup().)

It never frees up that memory though.  It is only used to pass to the
constructor of the nsFileSpec tmpFileSpec.

This ends up happening a good number of times.

[W] MLK: Memory leak of 34 bytes from 1 block allocated in PL_strdup
        Distribution of leaked blocks
                34 bytes from 1 block of 34 bytes (0x0980e908) allocation number
208960
        Allocation location
            malloc         [dbgheap.c:129]
            PL_strdup      [strdup.c:30]
            nsProfile::GetProfileDir(char const*,nsFileSpec *)
[nsProfile.cpp:359]
            nsProfile::GetCurrentProfileDir(nsFileSpec *) [nsProfile.cpp:822]
            nsBookmarksService::GetBookmarksFile(nsFileSpec *)
[nsBookmarksService.cpp:2976]
            nsBookmarksService::WriteBookmarks(nsIRDFDataSource *,nsIRDFResource
*) [nsBookmarksService.cpp:3220]
            nsBookmarksService::Flush(void) [nsBookmarksService.cpp:2954]
            nsBookmarksService::UpdateBookmarkLastVisitedDate(char const*)
[nsBookmarksService.cpp:2242]
            XPTC_InvokeByIndex [xptcinvoke.cpp:133]
            nsXPCWrappedNativeClass::CallWrappedMethod(JSContext
*,nsXPCWrappedNative *,XPCNativeMemberDescriptor
const*,CallMode::nsXPCWrappedNativeClass,UINT,long *,long *)
[xpcwrappednativeclass.cpp:507]
            WrappedNative_CallMethod(JSContext *,JSObject *,UINT,long *,long *)
[xpcwrappednativejsops.cpp:170]
            js_Invoke      [jsinterp.c:654]
            js_Interpret   [jsinterp.c:2228]
            js_Invoke      [jsinterp.c:670]
            js_InternalCall [jsinterp.c:747]

Comment 1

[Bruce Mitchener]

Cc'ing more appropriate people I think (hopefully!)

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Actually, I'm pretty sure you even need to strdup at all, because the nsFileSpec
constructor will copy for you.  You might even be able to use

  nsAutoString autoOrgProfileDir(orgProfileDir);
  nsFileSpec tmpFileSpec(autoOrgProfileDir);

because you ensure that profileDir's lifetime exceeds that of the tmpFileSpec.

Comment 3

[Alec Flett]

I just fixed this. Gotta love those nsXPIDLStrings.

Comment 4

[Grace Bush]

dev bug

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

What does ``dev bug'' mean?  Have we actually verified that this is fixed, by
checking subsequent purify runs or something?

Comment 6

[Grace Bush]

'dev bug' comment suggested for use by QA when bug is code fix by developer that
has no test case. I check lxr to see if code checked in by engineer who said he
did it

Comment 7

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Well, there's no test case attached, but we can surely determine if the fix is
proper by using the same tools that found the MLK in the first place.

What's the difference between RESOLVED/FIXED and VERIFIED/FIXED if there's no
actual verification?

(I'm sure someone would be happy to help you set up purify if you want to verify
this sort of bug.)

Comment 8

[Alec Flett]

reassigning QA contact to bruce, reopening.

Comment 9

[leger]

Moving all Profile Manager bugs to new Profile Manager Backend component.
Profile Manager component to be deleted.