Closed
Bug 1162293
Opened 9 years ago
Closed 9 years ago
Remove usage of |#ifdef PR_LOGGING| from widget
Categories
(Core :: Widget, defect)
Core
Widget
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: erahm, Assigned: erahm)
References
Details
Attachments
(2 files)
54.59 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
1.35 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
In bug 1161238 we plan on removing |--disable-logging| which makes |#ifdef PR_LOGGING| redundant.
Assignee | ||
Comment 1•9 years ago
|
||
PR_LOGGING is now always defined, we can remove #ifdefs checking for it.
Attachment #8602380 -
Flags: review?(nfroyd)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → erahm
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•9 years ago
|
||
If logging is enabled the result from |ToNewCString| will be leaked. Switched to |NS_LossyConvertUTF16toASCII| instead.
Attachment #8602381 -
Flags: review?(nfroyd)
Assignee | ||
Comment 3•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2f2fc3f5279
Updated•9 years ago
|
Attachment #8602380 -
Flags: review?(nfroyd) → review+
Comment 4•9 years ago
|
||
Comment on attachment 8602381 [details] [diff] [review] Part 2: Fix string leak in logging code Review of attachment 8602381 [details] [diff] [review]: ----------------------------------------------------------------- Does this choke if we use NSConvertUTF16toUTF8().get()? I should think that attempting not to lose data here is preferable. r=me assuming that works.
Attachment #8602381 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #4) > Comment on attachment 8602381 [details] [diff] [review] > Part 2: Fix string leak in logging code > > Review of attachment 8602381 [details] [diff] [review]: > ----------------------------------------------------------------- > > Does this choke if we use NSConvertUTF16toUTF8().get()? I should think that > attempting not to lose data here is preferable. r=me assuming that works. I'm maintaining parity w/ the previous implementation, |ToNewCString| does a lossy convert, whereas |ToNewUTF8String| would have mapped to |NS_ConvertUTF16toUTF8|.
Comment 6•9 years ago
|
||
(In reply to Eric Rahm [:erahm] from comment #5) > (In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #4) > > Does this choke if we use NSConvertUTF16toUTF8().get()? I should think that > > attempting not to lose data here is preferable. r=me assuming that works. > > I'm maintaining parity w/ the previous implementation, |ToNewCString| does a > lossy convert, whereas |ToNewUTF8String| would have mapped to > |NS_ConvertUTF16toUTF8|. Fair point! r=me unconditionally, then. :)
Assignee | ||
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/830ce4e36b47 https://hg.mozilla.org/integration/mozilla-inbound/rev/93eded78def9
https://hg.mozilla.org/mozilla-central/rev/830ce4e36b47 https://hg.mozilla.org/mozilla-central/rev/93eded78def9
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in
before you can comment on or make changes to this bug.
Description
•