Coverity found this: 98 UtilityWrite("<headerdisplayname>"); 4. alloc_fn: Storage is returned from allocation function LocalizeHeaderName. [show details] 5. var_assign: Assigning: l10nTagName = storage returned from this->LocalizeHeaderName(upCaseTag, tagName). 99 char *l10nTagName = LocalizeHeaderName(upCaseTag, tagName); 6. Condition !l10nTagName, taking false branch 7. Condition !*l10nTagName, taking true branch 100 if ( (!l10nTagName) || (!*l10nTagName) ) 8. Falling through to end of if statement 101 UtilityWrite(tagName); 102 else 103 { 104 UtilityWrite(l10nTagName); 105 PR_FREEIF(l10nTagName); 106 } 107 108 UtilityWrite(": "); 109 UtilityWrite("</headerdisplayname>"); 110 111 // Now write out the actual value itself and move on! 112 // 113 UtilityWrite(newValue); 114 UtilityWrite("</header>"); 115 116 NS_Free(upCaseTag); 9. Condition newValue, taking true branch 117 PR_FREEIF(newValue); 118 CID 1137493 (#1 of 1): Resource leak (RESOURCE_LEAK)10. leaked_storage: Variable l10nTagName going out of scope leaks the storage it points to. 119 return NS_OK; 120} Observation. 105 PR_FREEIF(l10nTagName); This can be simply moved out of else { } so that it is always executed before return. This is because I think PR_FREEIF() checks the nullness of its argument before callling PR_Free().

Here is a proposed patch.

Comment on attachment 8762567 [details] [diff] [review] Bug 1279247 - release |l10nTagName| always. Review of attachment 8762567 [details] [diff] [review]: ----------------------------------------------------------------- Don't forget to ask for review, or patches will just languish in bugzilla. I'll r this one now though. ::: mailnews/mime/emitters/nsMimeXmlEmitter.cpp @@ +105,2 @@ > } > + PR_FREEIF(l10nTagName); /* call this always */ please remove the comment

Patch fixed by removing comment. Carrying over: r=mkmelin+mozilla Thank you for noticing the missed review entry (!)