be consistent about speex_resampler symbol visibility

RESOLVED FIXED in mozilla34

Status

defect
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

Trunk
mozilla34
x86_64
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Currently the presence of speex/speex_resampler.h in dist/system_wrappers
means that some object files are looking for a hidden symbol and others a
default symbol.  This is because the header is included with "" and so another
exported header (in dist/include) finds the speex/speex_resampler.h in dist/include before dist/system_wrappers.  Visibility depends on the order of includes. (Fortunately a hidden definition seems to resolve a default visibility
undefined symbol.)

The convention is to use "" for includes of exported headers within mozilla.
<> would probably work but convention implies this is then a system header.
We don't want the system header, which may be different from the internal
header.

If GKMEDIAS_SHARED_LIBRARY is ever meant to work for non-WINNT platforms, then
I guess the approach should be to change to <speex/speex_resampler.h>
everywhere and wrap only when GKMEDIAS_SHARED_LIBRARY is set.  Its only necessary to use <> in exported headers, but code gets copied, so consistency is preferred.  I don't know of any intention to support GKMEDIAS_SHARED_LIBRARY elsewhere.

A similar approach is required if system includes are ever to be wrapped on
WINNT (c.f. bug 1008192).

If the GKMEDIAS_SHARED_LIBRARY is only ever WINNT and WRAP_SYSTEM_INCLUDES
will never be WINNT, then we could follow the "speex/speex_resampler.h"
convention everywhere.
Posted patch patchSplinter Review
Attachment #8449119 - Flags: review?(khuey)
Comment on attachment 8449119 [details] [diff] [review]
patch

The trunk/webrtc include is already a modification from trunk.
https://hg.mozilla.org/mozilla-central/rev/ad8a86bfd860#l9.12
https://hg.mozilla.org/mozilla-central/rev/98d22b0455f6
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
QA Whiteboard: [qa-]
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.