1280056 - (coverity) resource leak: mailnews/mime/emitters/nsMimeBaseEmitter.cpp: |l10nTagName| is not freed always.

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[ISHIKAWA, Chiaki]

Coverity found this:
|l10nTagName| is not freed in an early error return path.

 846
    14. alloc_fn: Storage is returned from allocation function LocalizeHeaderName. [show details]
    15. var_assign: Assigning: l10nTagName = storage returned from this->LocalizeHeaderName(newTagName.get(), field).
 847  char *l10nTagName = LocalizeHeaderName(newTagName.get(), field);
    16. Condition !l10nTagName, taking false branch
    17. Condition !*l10nTagName, taking true branch
 848  if ( (!l10nTagName) || (!*l10nTagName) )
    18. Falling through to end of if statement
 849    mHTMLHeaders.Append(field);
 850  else
 851  {
 852    mHTMLHeaders.Append(l10nTagName);
 853    PR_FREEIF(l10nTagName);
 854  }
 855
 856  mHTMLHeaders.Append(": ");
    19. Condition this->mFormat == nsMimeOutput::nsMimeMessageSaveAs, taking false branch
 857  if (mFormat == nsMimeOutput::nsMimeMessageSaveAs)
 858    mHTMLHeaders.Append("</b>");
 859  else
 860    mHTMLHeaders.Append("</div>");
 861
 862  // Now write out the actual value itself and move on!
 863  //
 864  mHTMLHeaders.Append(newValue);
 865  mHTMLHeaders.Append("</td>");
 866
 867  mHTMLHeaders.Append("</tr>");
 868
    20. Condition newValue, taking true branch
 869  PR_FREEIF(newValue);
    CID 450614 (#1 of 1): Resource leak (RESOURCE_LEAK)21. leaked_storage: Variable l10nTagName going out of scope leaks the storage it points to.
 870  return NS_OK;
 871}

Observation:

853    PR_FREEIF(l10nTagName);

this should be moved out of else { } so that it is always executed.

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: bug1280056_l10nTagName_leak.patch

I suppose the leak is always 0, but we might as well clear this one up.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/18910a0a17dd
coverity: mailnews/mime/emitters/nsMimeBaseEmitter.cpp: |l10nTagName| is not freed always. r=benc

Comment 3

[Magnus Melin [:mkmelin]]

Doesn't seem worth uplifting.