957002 - Stop including prtypes.h for endian macros

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Masatoshi Kimura [:emk]]

We don't have to pull in all prtype junks only for endian macros.

Comment 1

[(no longer active)]

We should just use MOZ_LITTLE_ENDIAN and MOZ_BIG_ENDIAN from mozilla/Endian.h instead of the NSPR equivalents, I think.

Comment 2

[Masatoshi Kimura [:emk]]

Oh, I din't know that.

Comment 3

[Masatoshi Kimura [:emk]]

Unfortunately MFBT is unusable in qcms because qcms is C code...

Comment 4

[(no longer active)]

(In reply to comment #3)
> Unfortunately MFBT is unusable in qcms because qcms is C code...

By doing some #ifdef __cplusplus stuff in Endian.h, you can make the part which defines MOZ_LITTLE_ENDIAN and MOZ_BIG_ENDIAN work in C code.  Please consider doing that if possible.  Thanks so much!

Comment 5

[Masatoshi Kimura [:emk]]

Attachment: Make MFBT endian macros usable from C code

Comment 6

[Masatoshi Kimura [:emk]]

Attachment: Stop including prtypes.h from gfx/qcms/

Looks like qcms is the only instance.

Comment 7

[Masatoshi Kimura [:emk]]

Try run: https://tbpl.mozilla.org/?tree=Try&rev=e61fd444724c

Comment 8

[Jeff Walden [:Waldo]]

Comment on attachment 8358759 [details] [diff] [review]
Make MFBT endian macros usable from C code

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

So Jeff now tells me on IRC that he'd rather just change qcms for this, actually, than have a dependency on mfbt.  So this isn't necessary/desirable, as we'd really rather not have to think about the C/C++ distinction whenever we can get away with it.

Comment 9

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #8)
> Comment on attachment 8358759 [details] [diff] [review]
> Make MFBT endian macros usable from C code
> 
> Review of attachment 8358759 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> So Jeff now tells me on IRC that he'd rather just change qcms for this,
> actually, than have a dependency on mfbt.  So this isn't
> necessary/desirable, as we'd really rather not have to think about the C/C++
> distinction whenever we can get away with it.

Copy & pasting the endian detection code from mfbt's Endian.h will be sufficient to replace the prtypes.h include.

Comment 10

[Masatoshi Kimura [:emk]]

Attachment: Stop including prtypes.h from qcms

Copy & pasted the logic from mfbt.
Also removed MOZ_QCMS check because this code no longer depends on other mozilla codebase.

Comment 11

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8370336 [details] [diff] [review]
Stop including prtypes.h from qcms

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

::: gfx/qcms/qcmstypes.h
@@ +4,1 @@
>  #include <stdint.h>

This can go, because we should get it at the bottom.

@@ -57,5 @@
> -#  include <sys/inttypes.h>
> -#else
> -#  include <stdint.h>
> -#endif
> -

We don't want to drop this hunk

Comment 12

[Masatoshi Kimura [:emk]]

I just copy & pasted the "logic" (just including stdint.h everywhere) for integer types, just like the logic for endian macros.
The removed hunk was enclosed by !MOZ_QCMS block which is untested, unmaintained (since 2009), and effectively dead.

>-#if defined (_SVR4) || defined (SVR4) || defined (__OpenBSD__) || defined (_sgi) || defined (__sun) || defined (sun) || defined (__digital__)
>-#  include <inttypes.h>

Is this hunk should be removed?

>-#elif defined (_MSC_VER)
>-typedef __int8 int8_t;
>-typedef unsigned __int8 uint8_t;
>-typedef __int16 int16_t;
>-typedef unsigned __int16 uint16_t;
>-typedef __int32 int32_t;
>-typedef unsigned __int32 uint32_t;
>-typedef __int64 int64_t;
>-typedef unsigned __int64 uint64_t;
>-#ifdef _WIN64
>-typedef unsigned __int64 uintptr_t;
>-#else
>-typedef unsigned long uintptr_t;
>-#endif

If I left the |#if defined(_MSC_VER)| hunk, the compiler warns with C4142 on Windows.

Comment 13

[Masatoshi Kimura [:emk]]

Attachment: Stop including prtypes.h from qcms, v3

Just removed |#ifdef MOZ_QCMS| and |#if defined (_MSC_VER)| hunks. Copy & pasting was not needed. Green on try:
https://tbpl.mozilla.org/?tree=Try&rev=0ef3485e28fb

If this is insufficient, please teach me what exact part should be copied from mfbt, what exact part should be removed, and what exact part should be left.

Comment 14

[Jeff Muizelaar [:jrmuizel]]

I've checked in a version of this with the _MSC_VER hunk kept.
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5be94737a83

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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