Closed
Bug 1103816
Opened 10 years ago
Closed 10 years ago
[gonk-l] update android_stub.h for api level 21
Categories
(Firefox OS Graveyard :: GonkIntegration, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
2.2 S2 (19dec)
People
(Reporter: seinlin, Assigned: mwu)
References
Details
Attachments
(1 file, 2 obsolete files)
1.37 KB,
patch
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Updated•10 years ago
|
Summary: [gonk-l] nss get compile error → [gonk-l] update android_stub.h for api level 21
Assignee | ||
Comment 1•10 years ago
|
||
Assignee: nobody → mwu
Assignee | ||
Comment 2•10 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b0803af1139e
Assignee | ||
Comment 3•10 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=42139d3d4e2e
Assignee | ||
Comment 4•10 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=33165857352b
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8537575 [details] [diff] [review] Add support for gonk-L to android_stub.h Review of attachment 8537575 [details] [diff] [review]: ----------------------------------------------------------------- ::: config/external/nss/Makefile.in @@ +212,5 @@ > OS_PTHREAD= \ > $(NULL) > > +ifeq ($(filter 21,$(ANDROID_VERSION)),) > +EXTRA_FLAGS := "-DRTLD_NOLOAD=0" RTLD_NOLOAD is actually a proper enum now, so defining RTLD_NOLOAD as zero means the compiler ends up seeing "0 = 4" when it opens dlfcn.h. http://androidxref.com/5.0.0_r2/xref/bionic/libc/include/dlfcn.h#52 Alternately we might be able to include dlfcn in the android stub and #define RTLD_NOLOAD 4 to avoid version checks. ::: security/manager/android_stub.h @@ +14,5 @@ > > #include <sys/cdefs.h> > #include <linux/kernel.h> > > +extern int getdtablesize(void); Android is deprecating this and removing it from their headers. It's still available for 32bit arm, but not in 64bit arm. We should probably switch NSS to sysconf() if we can, but that involves an NSS patch. Looks easy to do though - I'll follow up with a bug for that.
Attachment #8537575 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 6•10 years ago
|
||
On second thought, I don't think we can switch to sysconf so easily. Bionic's sysconf(_SC_OPEN_MAX) is hard coded to 256, but the actual default limit is 1024. glibc on the other hand, implements sysconf(_SC_OPEN_MAX) using getdtablesize, so there's no problems there.
Comment 7•10 years ago
|
||
Comment on attachment 8537575 [details] [diff] [review] Add support for gonk-L to android_stub.h Review of attachment 8537575 [details] [diff] [review]: ----------------------------------------------------------------- ::: config/external/nss/Makefile.in @@ +211,5 @@ > OS_RELEASE='2.6' \ > OS_PTHREAD= \ > $(NULL) > > +ifeq ($(filter 21,$(ANDROID_VERSION)),) This test is not future-proof. You should probably move the define in android_stub.h, since #if can do numeric comparisons there. @@ +212,5 @@ > OS_PTHREAD= \ > $(NULL) > > +ifeq ($(filter 21,$(ANDROID_VERSION)),) > +EXTRA_FLAGS := "-DRTLD_NOLOAD=0" There should be a comment to that effect in the code rather than in the bug. ::: security/manager/android_stub.h @@ +14,5 @@ > > #include <sys/cdefs.h> > #include <linux/kernel.h> > > +extern int getdtablesize(void); You should be able to use getrlimit(RLIMIT_NOFILE, ...)
Attachment #8537575 -
Flags: review?(mh+mozilla) → feedback-
Assignee | ||
Comment 8•10 years ago
|
||
Attachment #8537575 -
Attachment is obsolete: true
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 8538562 [details] [diff] [review] Add support for gonk-L to android_stub.h, v2 Review of attachment 8538562 [details] [diff] [review]: ----------------------------------------------------------------- Review comments addressed. I didn't put extra comments about RTLD_NOLOAD since moving the version check to the header makes it obvious.
Attachment #8538562 -
Flags: review?(mh+mozilla)
Comment 10•10 years ago
|
||
Comment on attachment 8538562 [details] [diff] [review] Add support for gonk-L to android_stub.h, v2 Review of attachment 8538562 [details] [diff] [review]: ----------------------------------------------------------------- ::: config/external/nss/Makefile.in @@ +211,5 @@ > OS_RELEASE='2.6' \ > OS_PTHREAD= \ > $(NULL) > > +ifneq ($(ANDROID_VERSION),) ifdef ANDROID_VERSION has the same meaning and is less obscure. OTOH, you could even do DEFAULT_GMAKE_FLAGS += $(addprefix -DANDROID_VERSION=,$(ANDROID_VERSION)), with no condition.
Attachment #8538562 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 11•10 years ago
|
||
Attachment #8538562 -
Attachment is obsolete: true
Assignee | ||
Comment 12•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/997e003830a8
Comment 13•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/997e003830a8
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S2 (19dec)
Comment 14•9 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #10) ... > ifdef ANDROID_VERSION has the same meaning and is less obscure. OTOH, you > could even do DEFAULT_GMAKE_FLAGS += $(addprefix > -DANDROID_VERSION=,$(ANDROID_VERSION)), with no condition. security/nss/lib/softoken/fipstokn.c fails to compile using NDK 10d here. It appears as though ANDROID_VERSION is not defined when including: security/manager/android_stub.h If the above is edited to explicitly contain '-DANDROID_VERSION=21' then it does build. fwiw 'ac_add_options --with-android-version=21' is included in the mozconfig file, yet config.status does not defined ANDROID_VERSION, rather ANDROID_TARGET_SDK is defined to be 21.
Flags: needinfo?(mh+mozilla)
Comment 15•9 years ago
|
||
(In reply to Douglas Crosher [:dougc] from comment #14) > fwiw 'ac_add_options --with-android-version=21' is included in the mozconfig > file, yet config.status does not defined ANDROID_VERSION, rather > ANDROID_TARGET_SDK is defined to be 21. ANDROID_VERSION is only defined in gonk builds.
Flags: needinfo?(mh+mozilla)
Comment 17•8 years ago
|
||
(In reply to Michael Wu [:mwu] from comment #6) > On second thought, I don't think we can switch to sysconf so easily. > Bionic's sysconf(_SC_OPEN_MAX) is hard coded to 256, but the actual default > limit is 1024. glibc on the other hand, implements sysconf(_SC_OPEN_MAX) > using getdtablesize, so there's no problems there. Just as a note, looks like ndk11 has removed getdtablesize from the 32-but headers too. NSS is failing - did you ever log an NSS bug for this?
Comment 18•8 years ago
|
||
Found it: https://bugzilla.mozilla.org/show_bug.cgi?id=1092004
You need to log in
before you can comment on or make changes to this bug.
Description
•