Remove usage of |#ifdef PR_LOGGING| from widget

RESOLVED FIXED in Firefox 40

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: erahm, Assigned: erahm)

Tracking

Trunk
mozilla40
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(2 attachments)

In bug 1161238 we plan on removing |--disable-logging| which makes |#ifdef PR_LOGGING| redundant.
PR_LOGGING is now always defined, we can remove #ifdefs checking for it.
Attachment #8602380 - Flags: review?(nfroyd)
Assignee: nobody → erahm
Status: NEW → ASSIGNED
If logging is enabled the result from |ToNewCString| will be leaked. Switched
to |NS_LossyConvertUTF16toASCII| instead.
Attachment #8602381 - Flags: review?(nfroyd)
Attachment #8602380 - Flags: review?(nfroyd) → review+
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+
(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|.
(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. :)
https://hg.mozilla.org/mozilla-central/rev/830ce4e36b47
https://hg.mozilla.org/mozilla-central/rev/93eded78def9
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.