Memory leak in nsEmlxHelperUtils::AddEmlxMessageToStream
RESOLVED
FIXED
in Thunderbird 13.0
Status
People
(Reporter: hiro, Assigned: hiro)
Tracking
({memory-leak})
Firefox Tracking Flags
(Not tracked)
Details
Attachments
(1 attachment, 3 obsolete attachments)
1.38 KB,
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
The return value of PR_smprintf should be freed by PR_smprintf_free.
(Assignee) | ||
Comment 1•8 years ago
|
||
Created attachment 558043 [details] [diff] [review] Proposed fix I have no build environment for Mac OS X, so I don't build comm-central on Mac OS X with this patch.
Attachment #558043 -
Flags: review?(dbienvenu)
Comment 2•8 years ago
|
||
Comment on attachment 558043 [details] [diff] [review] Proposed fix Fly-by comment: Doing this might be slightly nicer: nsCAutoString buf; buf.Adopt(PR_smprintf(X_MOZILLA_STATUS_FORMAT MSG_LINEBREAK, x_mozilla_flags)); Then you don't have to worry about what size the memory is.
(Assignee) | ||
Comment 3•8 years ago
|
||
Created attachment 558248 [details] [diff] [review] Revised patch Follow Mark's comment. > nsCAutoString buf; > buf.Adopt(PR_smprintf(X_MOZILLA_STATUS_FORMAT MSG_LINEBREAK, > x_mozilla_flags)); > > Then you don't have to worry about what size the memory is. I did not know the Adopt method. Thanks for the advice!
Attachment #558043 -
Attachment is obsolete: true
Attachment #558043 -
Flags: review?(dbienvenu)
Attachment #558248 -
Flags: review?(dbienvenu)
(Assignee) | ||
Comment 4•8 years ago
|
||
Created attachment 558249 [details] [diff] [review] Rewrite with Adopt for X-Mozilla-Status2 header Also use Adopt for X-Mozilla-Status2 header.
Assignee: nobody → hiikezoe
Attachment #558249 -
Flags: review?(dbienvenu)
(Assignee) | ||
Comment 5•8 years ago
|
||
Comment on attachment 558248 [details] [diff] [review] Revised patch Review of attachment 558248 [details] [diff] [review]: ----------------------------------------------------------------- ::: mailnews/import/applemail/src/nsEmlxHelperUtils.mm @@ +206,5 @@ > NS_ASSERTION(!buf.IsEmpty(), "printf error with X-Mozilla-Status header"); > if (buf.IsEmpty()) { > [pool release]; > return rv; > } The return value of this failure case seems to be NS_OK. Should return error value? Should I open a new bug for it?
Comment 6•7 years ago
|
||
bienvenu? (In reply to Hiroyuki Ikezoe (:hiro) from comment #5) > Comment on attachment 558248 [details] [diff] [review] > Revised patch > > Review of attachment 558248 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mailnews/import/applemail/src/nsEmlxHelperUtils.mm > @@ +206,5 @@ > > NS_ASSERTION(!buf.IsEmpty(), "printf error with X-Mozilla-Status header"); > > if (buf.IsEmpty()) { > > [pool release]; > > return rv; > > } > > The return value of this failure case seems to be NS_OK. Should return error > value? Should I open a new bug for it?
Severity: normal → minor
Keywords: mlk
Comment 7•7 years ago
|
||
Comment on attachment 558249 [details] [diff] [review] Rewrite with Adopt for X-Mozilla-Status2 header sorry for the delay - I think in out of memory, you want to return NS_ERROR_OUT_OF_MEMORY, not rv. Other than that, it looks OK (except for the question of whether the allocator aborts the app in the case of out of memory, in which case this code wouldn't be hit).
Attachment #558249 -
Flags: review?(dbienvenu) → review-
Comment 8•7 years ago
|
||
Comment on attachment 558248 [details] [diff] [review] Revised patch thx for the patch
Attachment #558248 -
Flags: review?(dbienvenu) → review+
(Assignee) | ||
Comment 9•7 years ago
|
||
Created attachment 591689 [details] [diff] [review] Return NS_ERROR_OUT_OF_MEMORY instead of rv.
Attachment #558249 -
Attachment is obsolete: true
Attachment #591689 -
Flags: review?(dbienvenu)
Comment 10•7 years ago
|
||
Comment on attachment 591689 [details] [diff] [review] Return NS_ERROR_OUT_OF_MEMORY instead of rv. thx for the patch...
Attachment #591689 -
Flags: review?(dbienvenu) → review+
(Assignee) | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 11•7 years ago
|
||
Comment on attachment 558248 [details] [diff] [review] Revised patch I understand this patch isn't required, so marking obsolete.
Attachment #558248 -
Attachment is obsolete: true
Updated•7 years ago
|
Summary: Memory leak in nsEmlxHelperUtils::AddEmlxMessa → Memory leak in nsEmlxHelperUtils::AddEmlxMessageToStream
Comment 12•7 years ago
|
||
Checked in: http://hg.mozilla.org/comm-central/rev/1717874b0963
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
You need to log in
before you can comment on or make changes to this bug.
Description
•