Closed
Bug 1092023
Opened 10 years ago
Closed 10 years ago
Android NDK r10c build failure: fatal error: sys/timeb.h: No such file or directory
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox34 unaffected, firefox35 fixed, firefox36 fixed)
RESOLVED
FIXED
Firefox 36
Tracking | Status | |
---|---|---|
firefox34 | --- | unaffected |
firefox35 | --- | fixed |
firefox36 | --- | fixed |
People
(Reporter: dougc, Assigned: gaston)
References
Details
(Whiteboard: [ndk-r10c])
Attachments
(1 file)
(deleted),
patch
|
eflores
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
../../../dist/system_wrappers/sys/timeb.h:3:28: fatal error: sys/timeb.h: No such file or directory
#include_next <sys/timeb.h>
It appears that sys/timeb.h in not included for android-21.
Reporter | ||
Updated•10 years ago
|
Whiteboard: [ndk-r10c]
Assignee | ||
Comment 1•10 years ago
|
||
Fwiw, if the error messages comes from media/gmp-clearkey/0.1/openaes, it's because the latter uses ftime() and struct timeb (cf http://pubs.opengroup.org/onlinepubs/7908799/xsh/systimeb.h.html) which isnt available everywhere, considered deprecated (cf http://man7.org/linux/man-pages/man3/ftime.3.html : "This function is obsolete. Don't use it.") and removed in posix.1-2008.
I'm hitting the same issue on OpenBSD (which removed timeb.h and ftime() a while ago) since openaes was added for GMP ClearKey in bug 1044742.
Blocks: 1044742
Assignee | ||
Comment 2•10 years ago
|
||
See https://github.com/kivy/python-for-android/issues/261 for a similar fallout on a different project. Guess we'll have to convert openaes to plain time() and gettimeofday() calls. Upstream at https://code.google.com/p/openaes/source/browse/src/oaes_lib.c still uses the deprecated funcs.
Assignee | ||
Comment 3•10 years ago
|
||
Of course MSVC doesnt support sys/time.h... see https://code.google.com/p/lz4/issues/detail?id=39
Assignee | ||
Comment 4•10 years ago
|
||
Hm, a library generating a seed for a RNG with such... "quality"... we surely have better in the tree, right ? im not even sure i want to touch a code like this:
static uint32_t oaes_get_seed()
....
gmTimer = gmtime( &timer.time );
_test = (char *) calloc( sizeof( char ), timer.millitm );
_ret = gmTimer->tm_year + 1900 + gmTimer->tm_mon + 1 + gmTimer->tm_mday +
gmTimer->tm_hour + gmTimer->tm_min + gmTimer->tm_sec + timer.millitm +
(uint32_t) ( _test + timer.millitm ) + getpid();
seriously ? Ppl still do that kind of things in 2014 ?
Assignee | ||
Comment 5•10 years ago
|
||
it also seems openaes builds rand.c, while it is unused since OAES_HAVE_ISAAC doesnt seem to be defined anywhere in headers or moz.build. So the !OAES_HAVE_ISAAC codepaths are used.
Converting it to gettimeofday() instead of ftime() is trivial and fixes the build, but that wont work on windows, and the existing code generating that seed is really horrible. Doesnt nss or nspr provide a decent & portable seed generation call ?
Assignee | ||
Comment 6•10 years ago
|
||
Edwin, what can we do about this issue ? it affects my aurora and central builds, and id like to avoid letting it propagate to beta..
status-firefox34:
--- → unaffected
status-firefox35:
--- → affected
status-firefox36:
--- → affected
Flags: needinfo?(edwin)
r=me removing rand.c from the build.
Flags: needinfo?(edwin)
Assignee | ||
Comment 8•10 years ago
|
||
sure, but what should we do about oaes_lib.c usage of ftime() to handroll a seed ? even if (and i didnt noticed the first time) the srand call in http://mxr.mozilla.org/mozilla-central/source/media/gmp-clearkey/0.1/openaes/oaes_lib.c#894 is commented out, ftime() call is built, and sys/timeb.h is included inconditionally.
Since the srand() call is commented out and that was the only caller of oaes_get_seed(), id suggest removing the declaration/definition of that function (such code is horrible), and removing sys/timeb.h inclusion. Does it look sane to you ? I'll prep up a patch for this.
Flags: needinfo?(edwin)
Assignee | ||
Comment 9•10 years ago
|
||
Went the easy route and commented the whole #ifdef/#endif block defining oaes_get_seed(), fixes build here. I can also remove the code, your call.
Assignee: nobody → landry
Attachment #8527289 -
Flags: review?(edwin)
Comment on attachment 8527289 [details] [diff] [review]
bug-1092023-no-timeb-header-comment-out-get-seed
Review of attachment 8527289 [details] [diff] [review]:
-----------------------------------------------------------------
Sweet. Also delete rand.c from the tree.
::: media/gmp-clearkey/0.1/openaes/oaes_lib.c
@@ +478,5 @@
>
> return OAES_RET_SUCCESS;
> }
>
> +/*
Does this even need to be commented out if the #ifdef is always false? Makes it easier to pull from upstream, the less we change.
Attachment #8527289 -
Flags: review?(edwin) → review+
Flags: needinfo?(edwin)
Assignee | ||
Comment 11•10 years ago
|
||
You mean comment out only the #else/#endif content ? That looks awkward...
Assignee | ||
Comment 12•10 years ago
|
||
And since we dont plan to enable OAES_HAVE_ISAAC, should rand.h be removed too ? For the sake of completeness, i wonder if we shouldnt remove all the other #if OAES_HAVE_ISAAC blocks in oaes_lib.c.. thoughts ?
Flags: needinfo?(edwin)
(In reply to Landry Breuil (:gaston) from comment #11)
> You mean comment out only the #else/#endif content ? That looks awkward...
Oops, didn't notice the #else.
(In reply to Landry Breuil (:gaston) from comment #12)
> And since we dont plan to enable OAES_HAVE_ISAAC, should rand.h be removed
> too ? For the sake of completeness, i wonder if we shouldnt remove all the
> other #if OAES_HAVE_ISAAC blocks in oaes_lib.c.. thoughts ?
Yes, removing rand.h is a good idea if it's unneeded. I'd rather not just take out the HAVE_ISAAC blocks, as it'd make updating difficult if we ever need to.
Flags: needinfo?(edwin)
Assignee | ||
Comment 14•10 years ago
|
||
The only thing that bugs me then is that oaes_lib.c includes rand.h if OAES_HAVE_ISAAC is defined, and rand.h is sooo "generic".. Oh well, i'll just remove rand.c and rand.h from the tree, that code is horrific - leaving OAES_HAVE_ISAAC blocks for the sake of merging from upstream..
https://hg.mozilla.org/integration/mozilla-inbound/rev/eaff9e176e42
Now i need to get this in aurora before the next merge...
Comment 15•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 16•10 years ago
|
||
Comment on attachment 8527289 [details] [diff] [review]
bug-1092023-no-timeb-header-comment-out-get-seed
Approval Request Comment
[Feature/regressing bug #]: 1044742
[User impact if declined]: Build failure on NDK r10c and OpenBSD
[Describe test coverage new/current, TBPL]: code was compiled but unused, commit fixes build issue. https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-central&revision=4631a7474d8a for the merge to m-c
[Risks and why]: NPOTB
Hoping to get this into 35...note that what i pushed also removes rand.c and rand.h.
Attachment #8527289 -
Flags: approval-mozilla-aurora?
Comment 17•10 years ago
|
||
Comment on attachment 8527289 [details] [diff] [review]
bug-1092023-no-timeb-header-comment-out-get-seed
approving, but for next time you are permitted to do a=NPOTB when that's the case :)
Attachment #8527289 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 18•10 years ago
|
||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•