1092023 - Android NDK r10c build failure: fatal error: sys/timeb.h: No such file or directory

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Douglas Crosher [:dougc]]

../../../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.

Comment 1

[Landry Breuil (:gaston)]

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.

Comment 2

[Landry Breuil (:gaston)]

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.

Comment 3

[Landry Breuil (:gaston)]

Of course MSVC doesnt support sys/time.h... see https://code.google.com/p/lz4/issues/detail?id=39

Comment 4

[Landry Breuil (:gaston)]

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 ?

Comment 5

[Landry Breuil (:gaston)]

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 ?

Comment 6

[Landry Breuil (:gaston)]

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..

Comment 7

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

r=me removing rand.c from the build.

Comment 8

[Landry Breuil (:gaston)]

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.

Comment 9

[Landry Breuil (:gaston)]

Attachment: bug-1092023-no-timeb-header-comment-out-get-seed

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.

Comment 10

[Edwin Flores [inactive from 2016-12-01] [:eflores] [: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.

Comment 11

[Landry Breuil (:gaston)]

You mean comment out only the #else/#endif content ? That looks awkward...

Comment 12

[Landry Breuil (:gaston)]

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 ?

Comment 13

[Edwin Flores [inactive from 2016-12-01] [:eflores] [: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.

Comment 14

[Landry Breuil (:gaston)]

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

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/eaff9e176e42

Comment 16

[Landry Breuil (:gaston)]

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.

Comment 17

[Lukas Blakk [:lsblakk] use ?needinfo]

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 :)

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a71cc988caf5