Last Comment Bug 665833 - Use nsCRTGlue rather than nsCRT in nsStyleStruct.cpp
: Use nsCRTGlue rather than nsCRT in nsStyleStruct.cpp
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla7
Assigned To: Joe Drew (not getting mail)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-20 22:03 PDT by Joe Drew (not getting mail)
Modified: 2011-06-22 16:30 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
use nsCRTGlue (2.76 KB, patch)
2011-06-20 22:03 PDT, Joe Drew (not getting mail)
dbaron: review+
Details | Diff | Splinter Review
v2: wrap strcmp in null-checks (3.63 KB, patch)
2011-06-21 13:40 PDT, Joe Drew (not getting mail)
dbaron: review+
Details | Diff | Splinter Review

Description Joe Drew (not getting mail) 2011-06-20 22:03:35 PDT
Created attachment 540677 [details] [diff] [review]
use nsCRTGlue

nsCRT::free gets screwed by mozalloc because there are various #undefs and #defines of free. Let's use nsCRTGlue in nsStyleStruct.cpp instead, so we can compile even if free gets #undefed from under us.
Comment 1 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-06-20 22:08:42 PDT
Comment on attachment 540677 [details] [diff] [review]
use nsCRTGlue

r=dbaron
Comment 2 Joe Drew (not getting mail) 2011-06-21 09:11:05 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/2783e9b15954
Comment 3 Mounir Lamouri (:mounir) 2011-06-21 10:04:37 PDT
Backed out due to orange, see:
http://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=2783e9b15954
Comment 4 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-06-21 13:15:56 PDT
Problem looks to be that nsCRT::strcmp is null-safe but NS_strcmp is not.  nsStyleContentData::operator== depends on the null-safety.
Comment 5 Joe Drew (not getting mail) 2011-06-21 13:40:41 PDT
Created attachment 540864 [details] [diff] [review]
v2: wrap strcmp in null-checks

This makes the tests that crashed on mozilla-incoming (sorry sorry sorry) pass on my local machine. I've pushed this to try.
Comment 6 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-06-21 14:13:26 PDT
Comment on attachment 540864 [details] [diff] [review]
v2: wrap strcmp in null-checks

I think we need safe_strcmp only in the second place you're using it, but you should test that :-)

r=dbaron with that
Comment 7 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-06-21 14:17:54 PDT
But I think you should do b-a rather than a-b.
Comment 8 Joe Drew (not getting mail) 2011-06-21 14:30:11 PDT
(In reply to comment #7)
> But I think you should do b-a rather than a-b.

To be clear, I just copied what PL_strcmp does. I don't care which I use :)
Comment 9 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-06-21 14:47:04 PDT
Er, sorry, a-b is correct.
Comment 10 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-06-21 14:48:25 PDT
(except that it doesn't work if the pointers are in the upper half of the integer space... but it doesn't really matter in this case)
Comment 11 Joe Drew (not getting mail) 2011-06-22 11:42:50 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/053ac4368a79

Note You need to log in before you can comment on or make changes to this bug.