Closed Bug 787933 Opened 12 years ago Closed 12 years ago

Stop using stdin types in IPC code

Categories

(Core :: IPC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: Ms2ger, Assigned: Ms2ger)

References

Details

Attachments

(4 files)

Actual stdint types are nicer.
Attached file Script
Generated by attachment 657850 [details].
Attachment #657851 - Flags: review?(benjamin)
Attached patch Part 2: FixesSplinter Review
Attachment #657853 - Flags: review?(benjamin)
openbsd builds are broken with these two patches, i'll work on a third one on top of them.
/home/landry/src/mozilla-central/ipc/chromium/src/base/singleton.h:118:77: error: no matching function for call to 'NoBarrier_Load(base::subtle::AtomicWord*&)'
/home/landry/src/mozilla-central/ipc/chromium/src/base/singleton.h:118:77: note: candidates are:
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:132:17: note: base::subtle::Atomic32 base::subtle::NoBarrier_Load(const volatile Atomic32*)
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:132:17: note:   no known conversion for argument 1 from 'base::subtle::AtomicWord* {aka long int*}' to 'const volatile Atomic32* {aka const volatile int*}'
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:236:17: note: base::subtle::Atomic64 base::subtle::NoBarrier_Load(const volatile Atomic64*) <near match>
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:236:17: note:   no known conversion for argument 1 from 'base::subtle::AtomicWord* {aka long int*}' to 'const volatile Atomic64* {aka const volatile long long int*}'
/home/landry/src/mozilla-central/ipc/chromium/src/base/singleton.h:125:65: error: no matching function for call to 'Acquire_CompareAndSwap(base::subtle::AtomicWord*&, int, const AtomicWord&)'
/home/landry/src/mozilla-central/ipc/chromium/src/base/singleton.h:125:65: note: candidates are:
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:71:17: note: base::subtle::Atomic32 base::subtle::Acquire_CompareAndSwap(volatile Atomic32*, base::subtle::Atomic32, base::subtle::Atomic32)
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:71:17: note:   no known conversion for argument 1 from 'base::subtle::AtomicWord* {aka long int*}' to 'volatile Atomic32* {aka volatile int*}'
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:199:17: note: base::subtle::Atomic64 base::subtle::Acquire_CompareAndSwap(volatile Atomic64*, base::subtle::Atomic64, base::subtle::Atomic64) <near match>
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:199:17: note:   no known conversion for argument 1 from 'base::subtle::AtomicWord* {aka long int*}' to 'volatile Atomic64* {aka volatile long long int*}'
/home/landry/src/mozilla-central/ipc/chromium/src/base/singleton.h:131:73: error: no matching function for call to 'Release_Store(base::subtle::AtomicWord*&, base::subtle::AtomicWord&)'
/home/landry/src/mozilla-central/ipc/chromium/src/base/singleton.h:131:73: note: candidates are:
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:126:13: note: void base::subtle::Release_Store(volatile Atomic32*, base::subtle::Atomic32)
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:126:13: note:   no known conversion for argument 1 from 'base::subtle::AtomicWord* {aka long int*}' to 'volatile Atomic32* {aka volatile int*}'
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:215:13: note: void base::subtle::Release_Store(volatile Atomic64*, base::subtle::Atomic64) <near match>
/home/landry/src/mozilla-central/ipc/chromium/src/base/atomicops_internals_x86_gcc.h:215:13: note:   no known conversion for argument 1 from 'base::subtle::AtomicWord* {aka long int*}' to 'volatile Atomic64* {aka volatile long long int*}'


First failure is a case of atomic type mismatching. Failure is on 64 bits, note the :

 48 #if defined(OS_OPENBSD) && !defined(ARCH_CPU_64_BITS)
 49 typedef Atomic32 AtomicWord;
 50 #else
 51 typedef intptr_t AtomicWord;
 52 #endif

snippet in atomicops.h.

so on OpenBSD/amd64, Atomic64 is int64_t (ie long long), and atomicWord is... intptr_t, ie long ? This time, it seems nspr types are not to blame..
How about if you change it to

#ifdef OS_OPENBSD
#ifdef ARCH_CPU_64_BITS
typedef Atomic64 AtomicWord;
#else
typedef Atomic32 AtomicWord;
#endif // ARCH_CPU_64_BITS
#else
typedef intptr_t AtomicWord;
#endif // OS_OPENBSD

?
Whoa, good catch. With that on top of your patchset, my amd64 builds are okay. Gotta check on my i386/32bit builder, but it's down atm, so don't hold the commit on that.

 // Use AtomicWord for a machine-sized pointer.  It will use the Atomic32 or
 // Atomic64 routines below, depending on your architecture.
-#if defined(OS_OPENBSD) && !defined(ARCH_CPU_64_BITS)
+#ifdef OS_OPENBSD
+#ifdef ARCH_CPU_64_BITS
+typedef Atomic64 AtomicWord;
+#else
 typedef Atomic32 AtomicWord;
+#endif // ARCH_CPU_64_BITS
 #else
 typedef intptr_t AtomicWord;
-#endif
+#endif // OS_OPENBSD
Comment on attachment 657851 [details] [diff] [review]
Part 1: Scripted changes

cjones, this won't hork us any more than we already are with the chromium import, right?
Attachment #657851 - Flags: superreview?(jones.chris.g)
Attachment #657851 - Flags: review?(benjamin)
Attachment #657851 - Flags: review+
Attachment #657853 - Flags: review?(benjamin) → review+
Note that patch 1 fails to apply on inbound tip.

applying bug-787933-1
patching file dom/ipc/ContentParent.h
Hunk #1 FAILED at 107
1 out of 1 hunks FAILED -- saving rejects to file dom/ipc/ContentParent.h.rej
patching file ipc/glue/RPCChannel.cpp
Hunk #2 FAILED at 663
1 out of 2 hunks FAILED -- saving rejects to file ipc/glue/RPCChannel.cpp.rej
Fixes the build on top of part 1&2 on OpenBSD.. might be debatable.
Attachment #659590 - Flags: review?(jones.chris.g)
Comment on attachment 657851 [details] [diff] [review]
Part 1: Scripted changes

Nope, not to my knowledge.
Attachment #657851 - Flags: superreview?(jones.chris.g) → superreview+
Attachment #659590 - Flags: review?(jones.chris.g) → review+
Depends on: 791567
Landed all three patches in one changeset, because they didn't build on their own:

https://hg.mozilla.org/mozilla-central/rev/513941951140
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.