1285432 - (coverity) resource leak: mailnews/imap/src/nsIMAPNamespace.cpp: |convertedFolderName| is not released in an early error path.

Status: NEW

(MailNews Core :: Networking: IMAP, defect)

Description

[ISHIKAWA, Chiaki]

Coverity found this:
|convertedFolderName| is not released in an early error return path.

444nsIMAPNamespace* nsIMAPNamespaceList::GetNamespaceForFolder(const char *hostName,
445                                                    const char *canonicalFolderName,
446                                                    char delimiter)
447{
   1. Condition !hostName, taking false branch
   2. Condition !canonicalFolderName, taking false branch
448  if (!hostName || !canonicalFolderName)
449    return nullptr;
450  
451  nsIMAPNamespace *resultNamespace = nullptr;
452  nsresult rv;
   3. alloc_fn: Storage is returned from allocation function AllocateServerFolderName. [show details]
   4. var_assign: Assigning: convertedFolderName = storage returned from nsIMAPNamespaceList::AllocateServerFolderName(canonicalFolderName, delimiter).
453  char *convertedFolderName = nsIMAPNamespaceList::AllocateServerFolderName(canonicalFolderName, delimiter);
454
   5. Condition convertedFolderName, taking true branch
455  if (convertedFolderName)
456  {
457
458    nsCOMPtr<nsIImapHostSessionList> hostSessionList = 
459             do_GetService(kCImapHostSessionListCID, &rv);
   6. Condition !!NS_FAILED_impl(rv), taking true branch
   7. Condition !!NS_FAILED_impl(rv), taking true branch
   8. Condition (bool)!!NS_FAILED_impl(rv), taking true branch
460    if (NS_FAILED(rv)) 
   CID 450417 (#1 of 1): Resource leak (RESOURCE_LEAK)9. leaked_storage: Variable convertedFolderName going out of scope leaks the storage it points to.
461      return nullptr;
462    hostSessionList->GetNamespaceForMailboxForHost(hostName, convertedFolderName, resultNamespace);
463    PR_Free(convertedFolderName);
464  }
465  else
466  {
467    NS_ASSERTION(false, "couldn't get converted folder name");
468  }
469  
470  return resultNamespace;
471}

Observation:

I  think we should simply release |convertedFolderName| in the early return case, too.

|convertedFolderName| is assigned a value from PL_strdup() within
ReplaceCharsInCopiedString().


1st:
char *nsIMAPNamespaceList::AllocateServerFolderName(const char *canonicalFolderName, char delimiter)
475{
   1. Condition delimiter, taking true branch
476  if (delimiter)
   2. alloc_fn: Storage is returned from allocation function ReplaceCharsInCopiedString. [show details]
   3. return_alloc_fn: Directly returning storage allocated by ReplaceCharsInCopiedString.
477    return nsImapUrl::ReplaceCharsInCopiedString(canonicalFolderName, '/', delimiter);
478  else
479    return NS_strdup(canonicalFolderName);
480}

2nd:
1295char *nsImapUrl::ReplaceCharsInCopiedString(const char *stringToCopy, char oldChar, char newChar)
1296{
1297  char oldCharString[2];
1298  *oldCharString = oldChar;
1299  *(oldCharString+1) = 0;
1300
    1. alloc_fn: Storage is returned from allocation function PL_strdup.
    2. var_assign: Assigning: translatedString = PL_strdup(stringToCopy).
1301  char *translatedString = PL_strdup(stringToCopy);
    3. identity_transfer: Passing translatedString as argument 1 to function PL_strstr, which returns an offset off that argument.
    4. noescape: Resource translatedString is not freed or pointed-to in function PL_strstr.
    5. var_assign: Assigning: currentSeparator = PL_strstr(translatedString, oldCharString).
1302  char *currentSeparator = PL_strstr(translatedString, oldCharString);
1303
    6. Condition currentSeparator, taking true branch
    10. Condition currentSeparator, taking true branch
    12. Condition currentSeparator, taking false branch
1304  while(currentSeparator)
1305  {
1306    *currentSeparator = newChar;
    7. identity_transfer: Passing currentSeparator + 1 as argument 1 to function PL_strstr, which returns an offset off that argument.
    8. var_assign: Assigning: currentSeparator = PL_strstr(currentSeparator + 1, oldCharString).
1307    currentSeparator = PL_strstr(currentSeparator+1, oldCharString);
    9. Jumping back to the beginning of the loop
    11. Jumping back to the beginning of the loop
1308  }
1309
    13. return_alloc: Returning allocated memory translatedString.
1310  return translatedString;
1311}