908201 - some imported webrtc code uses identifiers reserved for the language implementation

Status: RESOLVED INCOMPLETE

(Core :: WebRTC, defect)

Description

[Trevor Saunders (:tbsaunde)]

video_coding/main/source/timestamp_extrapolator.h uses _P and anything starting with _ is stuff only the language implementation should use, it turns out libstdc++ in the android ndk defines _P and this file doesn't compile when we use libstdc++ instead of stlport as the stl implementation.
spreadsortlib/spreadsort.hpp usese getchar as a template argument name.  Its also a libc function which may or not be fine per spec, but bionic seems to want to implement this function with #define getchar getc(stdin) which of course breaks things (though not with stlport because stlport helpfully undefines it.

Comment 1

[Makoto Kato [:m_kato]]

filed code review as https://webrtc-codereview.appspot.com/2167004

Comment 2

[Landry Breuil (:gaston)]

(In reply to Trevor Saunders (:tbsaunde) from comment #0)
> video_coding/main/source/timestamp_extrapolator.h uses _P and anything
> starting with _ is stuff only the language implementation should use, it
> turns out libstdc++ in the android ndk defines _P and this file doesn't
> compile when we use libstdc++ instead of stlport as the stl implementation.
> spreadsortlib/spreadsort.hpp usese getchar as a template argument name.  Its
> also a libc function which may or not be fine per spec, but bionic seems to
> want to implement this function with #define getchar getc(stdin) which of
> course breaks things (though not with stlport because stlport helpfully
> undefines it.

Fwiw the _P issue also hit OpenBSD and was fixed in https://bug807492.bugzilla.mozilla.org/attachment.cgi?id=788684
I'm also how hitting the same getchar issue on too, which is defined as a macro on OpenBSD:
/usr/include/stdio.h:#define    getchar()       getc(stdin)


In file included from /home/landry/mozilla-central/media/webrtc/trunk/webrtc/system_wrappers/source/sort.cc:29:0:                                              [4101/9696]/home/landry/mozilla-central/media/webrtc/trunk/webrtc/system_wrappers/source/../../../webrtc/system_wrappers/source/spreadsortlib/spreadsort.hpp:1034:106: error: macro "
getchar" passed 2 arguments, but takes just 0
       if(length(*curr) > char_offset && (length(*curr) <= (nextOffset + 1) || getchar((*curr), nextOffset) != getchar((*first), nextOffset))) {
                                                                                                          ^
/home/landry/mozilla-central/media/webrtc/trunk/webrtc/system_wrappers/source/../../../webrtc/system_wrappers/source/spreadsortlib/spreadsort.hpp:1034:139: error: macro "
getchar" passed 2 arguments, but takes just 0
       if(length(*curr) > char_offset && (length(*curr) <= (nextOffset + 1) || getchar((*curr), nextOffset) != getchar((*first), nextOffset))) {
                                                                                                                                           ^
/home/landry/mozilla-central/media/webrtc/trunk/webrtc/system_wrappers/source/../../../webrtc/system_wrappers/source/spreadsortlib/spreadsort.hpp:1089:22: error: macro "g
etchar" passed 2 arguments, but takes just 0
       if(getchar(x, u) < getchar(y, u))

Replacing all occurences of getchar by character as proposed by the webrtc-codereview patch seems to improve the situation. I have no idea why this didnt surface before though....

Comment 3

[Landry Breuil (:gaston)]

Fwiw i've just hit the getchar issue building aurora on powerpc with gcc 4.8.2, would be nice to have a proper fix upstreamed in webrtc. I know freebsd has a patch for this:

http://lists.freebsd.org/pipermail/freebsd-gecko/2013-April/003248.html

Cant figure out why this doesnt show up on other archs though

Comment 4

[Landry Breuil (:gaston)]

https://github.com/uggedal/aports/blob/master/main/xulrunner/getchar.patch other fix for uclibc...
https://code.google.com/p/webrtc/issues/detail?id=1901 issue upstream without activity

Comment 5

[Jan Beich]

(In reply to Landry Breuil (:gaston) from comment #3)
> http://lists.freebsd.org/pipermail/freebsd-gecko/2013-April/003248.html

That workraround only stays until 8.x is EoL'd. Recent FreeBSD and NetBSD versions are not affected because they #define getchar in stdio.h only for #ifndef __cplusplus case.

Bionic stdio.h bears OpenBSD CVS Id but #define getchar was recently removed.
https://github.com/android/platform_bionic/commit/f2cea02

Comment 6

[Landry Breuil (:gaston)]

In that case the issue seems to be that stdio.h is included instead of cstdio (which does #undef getchar), depending on the code being built with CC or CXX ?

Comment 7

[Landry Breuil (:gaston)]

After more exploration, this seems like a bug in gcc 4.8.2 fixed in 4.8.3 by http://gcc.gnu.org/viewcvs/gcc/branches/gcc-4_8-branch/libstdc%2B%2B-v3/include/c_global/cstdio?r1=196696&r2=204292&pathrev=204292

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58952

Testing with cstdio fixed here...

Comment 8

[Jan Beich]

libc++ doesn't #undef getchar but FreeBSD 10.0 and OS X 10.9 where libc++ used by default aren't affected. So, only platform/libc that want to |clang++ -stdlib=libc++| without "fixing" stdio.h would hit the error e.g., Bitrig and DragonFly.

https://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstdio?view=markup

Comment 9

[Dan Minor [:dminor]]

As far as I can tell, this is no longer an issue.