267298 - memory leak in /modules/libpref/src/nsPrefBranch.cpp

Status: RESOLVED INVALID

(SeaMonkey :: Preferences, defect)

Description

[Andrew Tong]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040809
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040809

I use valgrind to detect memory leak in mozilla and find a leak in nsPrefBranch.cpp

Here is the detail output of valgrind:

==1860== 16 bytes in 16 blocks are definitely lost in loss record 30 of
193
==1860==    at 0x1B901E74: malloc (vg_replace_malloc.c:131)
==1860==    by 0x1B9C34C4: PL_strdup (strdup.c:46)
==1860==    by 0x1D812131: GConfProxy::GetCharPref(char const*, char**)
(nsSystemPrefService.cpp:676)
==1860==    by 0x1D81139F: nsSystemPrefService::GetCharPref(char const*,
char**) (nsSystemPrefService.cpp:292)
==1860==    by 0x1CA152C7: nsPrefBranch::GetCharPref(char const*,
char**) (./modules/libpref/src/nsPrefBranch.cpp:215)
==1860==    by 0x1EF34596: mozSRoaming::GenerateLiPrefs()
(mozSRoaming.cpp:187)
==1860==    by 0x1EF348FC: mozSRoaming::EndSession()
(./extensions/sroaming/src/mozSRoaming.cpp:217)
==1860==    by 0x1DB28097: nsProfile::ShutDownCurrentProfile(unsigned)
(./profile/src/nsProfile.cpp:1371)
==1860==    by 0x8061C4A: DoOnShutdown()
(./xpfe/bootstrap/nsAppRunner.cpp:806)
==1860==    by 0x8064B4E: main (nsAppRunner.cpp:1792)
==1860==    by 0x1C0694C1: __libc_start_main (in /lib/i686/libc.so.6)

Reproducible: Always
Steps to Reproduce:
1. run mozilla under valgrind
2. visit some page(only visit your homepage is OK)
3. quit mozilla

Comment 1

[Andrew Tong]

Attachment: free '\0' string

I free the '\0' string which is ignored before.

Comment 2

[Bernard Alleysson]

The patch looks strange, it looks like it's against an old nsPrefBranch.cpp
revision.

Comment 3

[Andrew Tong]

NS_IMETHODIMP nsPrefBranch::GetCharPref(const char *aPrefName, char **_retval)
{
  const char *pref;
  nsresult   rv;

  rv = getValidatedPrefName(aPrefName, &pref);
  if (NS_SUCCEEDED(rv)) {
    if (checkMozPrefsKey(aPrefName) && !mIsDefault) {
      PRBool useSystemPrefsValue = PR_FALSE;
      rv = GetBoolPref(UseSystemPref,&useSystemPrefsValue);
      if (useSystemPrefsValue) {
        nsCOMPtr<nsIPrefBranch> sysPrefService =
                       do_GetService(sSysPrefService, &rv);
      if (NS_SUCCEEDED(rv) && sysPrefService) {
        rv =sysPrefService->GetCharPref(aPrefName,_retval);
        if (strlen(*_retval) != 0)
          return rv;
  +     else
  +       free(*_retval);
        // strlen(str) == 0 does not mean str == NULL
        // if str == '\0' (str has one byte memory)
        // strlen('\0') equal 0
        // so we must free *_retval before give it new value
        // in function PREF_CopyCharPref.
      }
     }
    }
    rv = PREF_CopyCharPref(pref, _retval, mIsDefault);
  }
  return rv;
}

Here is the function who malloc momory for *_retval

PR_IMPLEMENT(char *)
PL_strdup(const char *s)
{
    char *rv;
    PRUint32 l;

    l = PL_strlen(s); 

    rv = (char *)malloc(l+1); // it malloc l+1 byte, not l byte.
    if( (char *)0 == rv ) return rv;

    if( (const char *)0 == s )
        *rv = '\0';
    else
        (void)PL_strcpy(rv, s);

    return rv;
}

Comment 4

[Andrew Tong]

This bug doesn't exist in community mozilla, sorry.
Thanks for your attention.

Comment 5

[Bernard Alleysson]

Please clear the patch pending review requests, thanks.