Status

()

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: dougt, Assigned: derf)

Tracking

Trunk
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

8 years ago
/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)
Reporter

Updated

8 years ago
Assignee: nobody → tterribe
looks like a regression from bug 634557
Blocks: 634557
OS: Linux → Android
Hardware: x86_64 → ARM
Assignee

Comment 2

8 years ago
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.
Attachment #526786 - Flags: review?(doug.turner)
Reporter

Comment 3

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

not a patch.
Assignee

Comment 4

8 years ago
Huh, I must have somehow clicked on aclocal.m4 instead. Let me try this again.
Attachment #526786 - Attachment is obsolete: true
Attachment #526786 - Flags: review?(doug.turner)
Attachment #526841 - Flags: review?(doug.turner)
Reporter

Comment 5

8 years ago
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
Attachment #526841 - Flags: review?(doug.turner) → review+
Assignee

Comment 6

8 years ago
(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)
Reporter

Comment 7

8 years ago
no idea.  maybe the top of the file for now?
Assignee

Comment 8

8 years ago
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.
Reporter

Comment 9

8 years ago
http://hg.mozilla.org/mozilla-central/rev/deee3852caaf
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.