__real___pthread_cond_timedwait and __pthread_cond_timedwait are not found.

Cervantes, Could you have a look to this patch? I am not sure if it is reasonable to disable them for api level 21 (gonk-l).

Comment on attachment 8528150 [details] [diff] [review] bug-1103827.patch I am not a fan of ANDROID_VERSION checks in the code base. I searched usage of __pthread_cond_timedwait() and no one is (and shouldn't be) using this nonstandard function. I think we should just remove __wrap_pthread_cond_timedwait() and declarations of __pthread_cond_timedwait() and __real___pthread_cond_timedwait().

Cyu, I verified on try server, remove __real___pthread_cond_timedwait will cause compile error. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9eebfc65c9f2 So, I think we should only remove __wrap_pthread_cond_timedwait() and declarations of __pthread_cond_timedwait(). How do you think?

Remove __wrap_pthread_cond_timedwait() and declarations of __pthread_cond_timedwait() will also breaks the build. https://tbpl.mozilla.org/?tree=Try&rev=bd712aeceae6

(In reply to Kai-Zhen Li [:seinlin] from comment #3) > Cyu, > > I verified on try server, remove __real___pthread_cond_timedwait will cause > compile error. > https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9eebfc65c9f2 > > So, I think we should only remove __wrap_pthread_cond_timedwait() and > declarations of __pthread_cond_timedwait(). > > How do you think? Yeah, please go ahead with removal of __pthread_cond_timed_wait() and its wrapper. It's not supposed to be used by any means. Also please remove the LD flag --wrap=__pthread_cond_timedwait in configure.in.

Is __pthread_cond_timedwait different from pthread_cond_timedwait? Are you sure it's not used? I think we have uses of it in the code base.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #6) > Is __pthread_cond_timedwait different from pthread_cond_timedwait? Are you __pthread_cond_timedwait() is not part of the pthreads standard. glibc has it as a strong alias: strong_alias (__pthread_cond_timedwait, pthread_cond_timedwait) so it's functionally identical. In bionic libc before L, __pthread_cond_timedwait() has an extra parameter for specify the clock to use. So even the 2 libraries has __pthread_cond_timedwait() the signatures are different. After L bionic just declares it file static. I think it's accceptable. Given that I don't think we should ever call __pthread_cond_timedwait(). > sure it's not used? I think we have uses of it in the code base. No at least from DXR.

Cyu, Could you review this patch? I verified on try server and it will not break any build. Try result: https://tbpl.mozilla.org/?tree=Try&rev=9e7e1fe9b5ec

Comment on attachment 8531868 [details] [diff] [review] bug-1103827.patch Review of attachment 8531868 [details] [diff] [review]: ----------------------------------------------------------------- r- because you also removed __wrap_pthread_cond_timedwait() that we need. Please only remove __real___pthread_cond_timedwait() and __wrap___pthread_cond_timed_wait() (notice the number of underscores). ::: configure.in @@ +7288,1 @@ > fi You need to keep --wrap=pthread_cond_timedwait ::: mozglue/build/Nuwa.cpp @@ -53,5 @@ > int timeout); > int __real_pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mtx); > -int __real_pthread_cond_timedwait(pthread_cond_t *cond, > - pthread_mutex_t *mtx, > - const struct timespec *abstime); You need to keep this declaration. @@ -1121,5 @@ > - THREAD_FREEZE_POINT2_VIP(); > - > - return rv; > -} > - You need to keep this wrapper.

Cervantes, I updated a new patch, could you have a look?

Comment on attachment 8535531 [details] [diff] [review] bug-1103827.patch Review of attachment 8535531 [details] [diff] [review]: ----------------------------------------------------------------- Please also clean up --wrap=__pthread_cond_timedwait in configure.in.

Cervantes, --wrap=__pthread_cond_timedwait is removed. Could you review this patch? Thanks! Try result: https://tbpl.mozilla.org/?tree=Try&rev=07be5b6c19eb