mimemsg.cpp might leak memory in some instances

RESOLVED FIXED in Thunderbird 40.0

Status

MailNews Core
MIME
--
minor
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: LABORDE--SCHUMACHER Nicolaï, Assigned: LABORDE--SCHUMACHER Nicolaï)

Tracking

({clang-analyzer})

Trunk
Thunderbird 40.0
clang-analyzer

Thunderbird Tracking Flags

(thunderbird40 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
Created attachment 8579302 [details]
mimemsg.cpp.diff

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_2) AppleWebKit/600.4.10 (KHTML, like Gecko) Version/8.0.4 Safari/600.4.10

Steps to reproduce:

Script found possible memory leak.


Actual results:

A variable is not de-allocated.


Expected results:

The variable should be de-allocated in order to prevent the memory leak.
(Assignee)

Updated

3 years ago
Mentor: mozilla
Keywords: checkin-needed
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [lang=cpp]
(Assignee)

Updated

3 years ago
Attachment #8579302 - Flags: review?(standard8)
Attachment #8579302 - Flags: review?(mozilla)
Please don't set the keywords/whiteboard/mentor fields unless you know what they are for. In the case of mentor, that person needs to actually agree to mentor.
Mentor: mozilla
Keywords: checkin-needed
Whiteboard: [lang=cpp]
Comment on attachment 8579302 [details]
mimemsg.cpp.diff

Please can you submit a diff -u patch with full file headers. Please see https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch for the general process here and for how to create patches.

Your better option for a reviewer is jcranmer (enter ":jcranmer" in the review requestee box), as he knows that code best.
Attachment #8579302 - Flags: review?(standard8)
Attachment #8579302 - Flags: review?(mozilla)
Component: Untriaged → MIME
Product: Thunderbird → MailNews Core
Summary: Memory leak → mimemsg.cpp might leak memory in some instances
(Assignee)

Comment 3

3 years ago
Created attachment 8579345 [details] [diff] [review]
mimemsg.cpp.diff
Attachment #8579302 - Attachment is obsolete: true
Attachment #8579345 - Flags: review?(Pidgeot18)

Comment 4

3 years ago
(assuming this leak does not impact TB usage in a significant way)
Severity: normal → minor
Comment on attachment 8579345 [details] [diff] [review]
mimemsg.cpp.diff

Review of attachment 8579345 [details] [diff] [review]:
-----------------------------------------------------------------

I'm sorry to have to r- this patch, but the diff context is insufficient for me to establish where the context of this code is, and the patch does not apply to code.

Please follow the steps at <https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F> to make a patch and upload to this bug

::: mimemsg.cpp
@@ +429,5 @@
>  
>        /* Emit a normal header block. */
>        status = MimeMessage_write_headers_html(obj);
> +      if (status < 0)
> +			{

I will tell you this: the coding style here is to use
if (foo)
{
  // Two-space indent! No more, no fewer.
  // And tabs are forbidden!
}
Attachment #8579345 - Flags: review?(Pidgeot18) → review-
(Assignee)

Comment 6

3 years ago
Created attachment 8582001 [details] [diff] [review]
patch.diff
Attachment #8579345 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8582001 - Flags: review?(Pidgeot18)
Keywords: clang-analyzer
Attachment #8582001 - Flags: review?(Pidgeot18) → review+
Assignee: nobody → nicolai.labordeschumacher
Keywords: checkin-needed

Comment 7

3 years ago
https://hg.mozilla.org/comm-central/rev/582e4c7d9525
Status: UNCONFIRMED → RESOLVED
Last Resolved: 3 years ago
status-thunderbird40: --- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 40.0
You need to log in before you can comment on or make changes to this bug.