(coverity) resouce leak: mailnews/mime/emitters/nsMimeXmlEmitter.cpp : |l10nTagName| not released all the time.

RESOLVED FIXED in Thunderbird 50.0

Status

MailNews Core
MIME
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ISHIKAWA, Chiaki, Assigned: ISHIKAWA, Chiaki)

Tracking

(Blocks: 1 bug, {coverity})

Trunk
Thunderbird 50.0
coverity

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: CID 1137493, URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
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().
(Assignee)

Comment 1

2 years ago
Created attachment 8762567 [details] [diff] [review]
Bug 1279247 - release |l10nTagName| always.

Here is a proposed patch.
(Assignee)

Updated

2 years ago
Attachment #8762567 - Attachment description: Bug 1279247 - release |l10nTagName| always.coverity-1137493.patch → Bug 1279247 - release |l10nTagName| always.

Comment 2

2 years ago
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
Attachment #8762567 - Flags: review+

Updated

2 years ago
Assignee: nobody → ishikawa
(Assignee)

Comment 3

2 years ago
Created attachment 8762602 [details] [diff] [review]
Bug 1279247 - release |l10nTagName| always. comment removed. carrying over  r=mkmelin+mozilla

Patch fixed by removing comment. Carrying over: r=mkmelin+mozilla

Thank you for noticing the missed review entry (!)
Attachment #8762567 - Attachment is obsolete: true
Attachment #8762602 - Flags: review+

Comment 4

2 years ago
https://hg.mozilla.org/comm-central/rev/b46028761bcef481600afd2364d2e636040a884c
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
You need to log in before you can comment on or make changes to this bug.