Closed Bug 1162293 Opened 6 years ago Closed 6 years ago

Remove usage of |#ifdef PR_LOGGING| from widget

Categories

(Core :: Widget, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: erahm, Assigned: erahm)

References

Details

Attachments

(2 files)

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. :)
You need to log in before you can comment on or make changes to this bug.