849598 - snappy is broken on big-endian

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Cameron Kaiser [:spectre]]

TenFourFox: Endian Little Hate We(tm)

There is all this lovely big-endian compliant code in Snappy, but Firefox uses none of it. Patch follows.

Comment 1

[Cameron Kaiser [:spectre]]

Attachment: Define WORDS_BIGENDIAN on big-endian platforms

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 723158 [details] [diff] [review]
Define WORDS_BIGENDIAN on big-endian platforms

Review of attachment 723158 [details] [diff] [review]:
-----------------------------------------------------------------

This belongs in other-licenses/snappy/snappy-stubs-public.h  That's the only file we modify from upstream.

Comment 3

[Cameron Kaiser [:spectre]]

Attachment: Define WORDS_BIGENDIAN on big-endian platforms (v2)

Okie dokie, moved to snappy-stubs-public.h.in.

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 723209 [details] [diff] [review]
Define WORDS_BIGENDIAN on big-endian platforms (v2)

Review of attachment 723209 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, khuey is incorrect. Please see the README file for the explanation, but we need to update mozilla/other-licenses/snappy/snappy-stubs-public.h with these changes.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to ben turner [:bent] from comment #4)
> Comment on attachment 723209 [details] [diff] [review]
> Define WORDS_BIGENDIAN on big-endian platforms (v2)
> 
> Review of attachment 723209 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Sorry, khuey is incorrect. Please see the README file for the explanation,
> but we need to update mozilla/other-licenses/snappy/snappy-stubs-public.h
> with these changes.

That's what I said, he just didn't read carefully enough ;-)

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

/me grabs more coffee

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 723209 [details] [diff] [review]
Define WORDS_BIGENDIAN on big-endian platforms (v2)

You need to change other-licenses/snappy/snappy-stubs-public.h, not other-licenses/snappy/src/snappy-stubs-public.h.in.  The latter is a direct copy of upstream, and changes are only reflected in our (modified) other-licenses/snappy/snappy-stubs-public.h by hand.

Comment 8

[Cameron Kaiser [:spectre]]

Attachment: Define WORDS_BIGENDIAN on big-endian platforms (v3)

The one time I try to be clever ... no wonder it wasn't sticking in my rebuild.

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 723211 [details] [diff] [review]
Define WORDS_BIGENDIAN on big-endian platforms (v3)

Review of attachment 723211 [details] [diff] [review]:
-----------------------------------------------------------------

And the third time is the charm.

What branches do you need this on?

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 723211 [details] [diff] [review]
Define WORDS_BIGENDIAN on big-endian platforms (v3)

Review of attachment 723211 [details] [diff] [review]:
-----------------------------------------------------------------

::: other-licenses/snappy/snappy-stubs-public.h
@@ +33,2 @@
>  // which is a public header. Instead, snappy-stubs-public.h is generated by
>  // from snappy-stubs-public.h.in at configure time.

So this is a lie?

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to :Ms2ger from comment #10)
> Comment on attachment 723211 [details] [diff] [review]
> Define WORDS_BIGENDIAN on big-endian platforms (v3)
> 
> Review of attachment 723211 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: other-licenses/snappy/snappy-stubs-public.h
> @@ +33,2 @@
> >  // which is a public header. Instead, snappy-stubs-public.h is generated by
> >  // from snappy-stubs-public.h.in at configure time.
> 
> So this is a lie?

In our build system, yes.  s/configure time/when we update snappy from upstream/

Comment 12

[Cameron Kaiser [:spectre]]

> What branches do you need this on?

Theoretically it affects everything from ESR17 on up -- your call. I'm fine with it just going in central though (I already have it checked into our internal changesets).

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a61fc19b0b24

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/a61fc19b0b24