650834 - Android debug build bustage

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Doug Turner (:dougt)]

/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?)]

looks like a regression from bug 634557

Comment 2

[Timothy B. Terriberry (:derf)]

Attachment: 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)]

Comment on attachment 526786 [details] [diff] [review]
Add casts to pointer differences in assertion

not a patch.

Comment 4

[Timothy B. Terriberry (:derf)]

Attachment: 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)]

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)]

(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)]

no idea.  maybe the top of the file for now?

Comment 8

[Timothy B. Terriberry (:derf)]

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)]

http://hg.mozilla.org/mozilla-central/rev/deee3852caaf