Last Comment Bug 650834 - Android debug build bustage
: Android debug build bustage
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: ---
Assigned To: Timothy B. Terriberry (:derf)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks: 634557
  Show dependency treegraph
 
Reported: 2011-04-18 09:59 PDT by Doug Turner (:dougt)
Modified: 2011-04-19 15:26 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add casts to pointer differences in assertion (975 bytes, patch)
2011-04-18 12:27 PDT, Timothy B. Terriberry (:derf)
no flags Details | Diff | Splinter Review
Add casts to pointer differences in assertion (1.41 KB, patch)
2011-04-18 15:03 PDT, Timothy B. Terriberry (:derf)
doug.turner: review+
Details | Diff | Splinter Review

Description Doug Turner (:dougt) 2011-04-18 09:59:26 PDT
/builds/mozilla-central/gfx/ycbcr/ycbcr_to_rgb565.cpp: In function 'void mozilla::gfx::ScaleYCbCrToRGB565(const PRUint8*, const PRUint8*, const PRUint8*, PRUint8*, int, int, int, int, int, int, int, int, int, mozilla::gfx::YUVType, mozilla::gfx::ScaleFilter)':
/builds/mozilla-central/gfx/ycbcr/ycbcr_to_rgb565.cpp:379: error: invalid operands of types 'const PRUint8*' and 'int' to binary 'operator&'
/builds/mozilla-central/gfx/ycbcr/ycbcr_to_rgb565.cpp:379: error: invalid operands of types 'const PRUint8*' and 'int' to binary 'operator&'
/builds/mozilla-central/gfx/ycbcr/ycbcr_to_rgb565.cpp:379: error: invalid operands of types 'const PRUint8*' and 'int' to binary 'operator&'
make[6]: *** [ycbcr_to_rgb565.o] Error 1


Usnig arm-eabi-gcc (GCC) 4.4.0 (ndk-4c)
Comment 1 Brad Lassey [:blassey] (use needinfo?) 2011-04-18 10:07:07 PDT
looks like a regression from bug 634557
Comment 2 Timothy B. Terriberry (:derf) 2011-04-18 12:27:08 PDT
Created attachment 526786 [details] [diff] [review]
Add casts to pointer differences in assertion

This was meant to convert a pointer to a ptrdiff_t by taking its difference from NULL, but without the explicit casts it was just treating NULL as 0, yielding the original pointer back.
Comment 3 Doug Turner (:dougt) 2011-04-18 12:53:07 PDT
Comment on attachment 526786 [details] [diff] [review]
Add casts to pointer differences in assertion

not a patch.
Comment 4 Timothy B. Terriberry (:derf) 2011-04-18 15:03:04 PDT
Created attachment 526841 [details] [diff] [review]
Add casts to pointer differences in assertion

Huh, I must have somehow clicked on aclocal.m4 instead. Let me try this again.
Comment 5 Doug Turner (:dougt) 2011-04-18 15:42:58 PDT
Comment on attachment 526841 [details] [diff] [review]
Add casts to pointer differences in assertion

Does it make sense to define |(PRUint8 *)NULL)&15| somewhere.  either way:

r=wfm
Comment 6 Timothy B. Terriberry (:derf) 2011-04-18 16:00:42 PDT
(In reply to comment #5)
> Does it make sense to define |(PRUint8 *)NULL)&15| somewhere.  either way:

Maybe! For all I know there already is a macro for these kinds of tests. If so, where would it go? (so I can look to see if it's already there)
Comment 7 Doug Turner (:dougt) 2011-04-18 16:06:41 PDT
no idea.  maybe the top of the file for now?
Comment 8 Timothy B. Terriberry (:derf) 2011-04-18 16:27:38 PDT
It looks like there are currently at least five in the tree: jpeg (IS_ALIGNED), jemalloc (ALIGN_ADDR2OFFSET), libffi (is_aligned), sqlite (EIGHT_BYTE_ALIGNMENT), freetype2 (GXV_32BIT_ALIGNMENT_VALIDATE). But none of them at the top level where everyone could use them.

Given that, I don't think it's worth it for two lines in an assert. It's better to see what the code is actually doing, where it's doing it.
Comment 9 Doug Turner (:dougt) 2011-04-19 15:26:02 PDT
http://hg.mozilla.org/mozilla-central/rev/deee3852caaf

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