[hipl-dev] Re: [Merge] lp:~hipl-core/hipl/android-port-new into lp:hipl
- From: Miika Komu <mkomu@xxxxxxxxx>
- To: hipl-dev@xxxxxxxxxxxxx
- Date: Tue, 22 Oct 2013 14:58:19 +0300
Hi,
On 10/22/2013 01:25 PM, Juhani Toivonen wrote:
Auto-inserting stuff into .bashrc is evil.
Ok, I'll make it ask before touching .bashrc .
evil, but necessary to make the Android build work (feel free to suggest
a better working solution). I think the warning should be enough.
--- android/android.h 1970-01-01 00:00:00 +0000
+++ android/android.h 2013-10-18 07:49:35 +0000
@@ -0,0 +1,44 @@
+
+#ifndef HIPL_ANDROID
+#define HIPL_ANDROID
missing _H
Ok, will add _H.
+#include <stdint.h>
+#include <inttypes.h>
You don't need inttypes.h.
When removed, build breaks in libcore/conf.c not finding SCNu32.
My idea was to try to keep android-specific changes to source files at minimal,
to let them get everything they need by just including android.h. I can remove
that from there and add it inside the #ifdef CONFIG_HIP_ANDROID -blocks in the
files that need it.
I think the current solution minimizes Android hacks, please keep it
unless Diego suggests a better solution.
+/* Logging */
+#define ALOGE printf
+
+/* Magic numbers */
+#define PROPERTY_KEY_MAX 32
+
+/* Networking */
+#define ICMP6_FILTER 1
+#define HOST_NAME_MAX 64
+typedef uint16_t in_port_t;
Android lacks all of this?
https://android.googlesource.com/platform/prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.7-4.6/+/android-4.1.1_r6/sysroot/usr/include/bits/local_lim.h
suggests that at least HOST_NAME_MAX exists, and
http://code.google.com/p/android/issues/detail?id=14376
would make it seem as if in_port_t is available…
Currently we are building against the Android NDK. I grepped through it and
still believe all of them are necessary.
jmttoivo@charlie:~/android_tools/toolchain$ find . -type f -print0 |xargs -0
grep PROPERTY_KEY_MAX
jmttoivo@charlie:~/android_tools/toolchain$ find . -type f -print0 |xargs -0
grep ICMP6_FILTER
./sysroot/usr/include/netinet/icmp6.h:#define
ICMP6_FILTER_SETPASSALL(filterp) \
./sysroot/usr/include/netinet/icmp6.h:#define
ICMP6_FILTER_SETBLOCKALL(filterp) \
./sysroot/usr/include/netinet/icmp6.h:#define ICMP6_FILTER_SETPASS(type,
filterp) \
./sysroot/usr/include/netinet/icmp6.h:#define ICMP6_FILTER_SETBLOCK(type,
filterp) \
./sysroot/usr/include/netinet/icmp6.h:#define ICMP6_FILTER_WILLPASS(type,
filterp) \
./sysroot/usr/include/netinet/icmp6.h:#define ICMP6_FILTER_WILLBLOCK(type,
filterp) \
jmttoivo@charlie:~/android_tools/toolchain$ find . -type f -print0 |xargs -0
grep HOST_NAME_MAX
jmttoivo@charlie:~/android_tools/toolchain$ find . -type f -print0 |xargs -0
grep in_port_t
./lib/python2.7/lib2to3/tests/data/infinite_recursion.py:in_port_t = __uint16_t
./lib/python2.7/lib2to3/tests/data/infinite_recursion.py:
'dh_method', 'bio_f_buffer_ctx_struct', 'in_port_t',
./sysroot/usr/include/sys/_types.h:typedef __uint16_t __in_port_t; /* IP
port type */
The bare ICMP6_FILTER itself was defined nowhere. With in_port_t I'm not sure
how to use that __in_port_t in a clean way, i.e. not changing its instances in
code to __in_port_t. The other two didn't bring up anything.
The first one says it is a "New 64-bit prebuilt linux toolchain to build SDK",
not sure if that works for us and after how much effort.
The latter link seems to suggest that in_port_t was not found at all and
in_addr_t was found in the wrong place.
Android porting of C apps is hacky, nasty and dirty. I think the current
proposal pretty much minimizes and isolates the changes in HIPL source
code. Juhani has tried several approaches and building against the NDK
was the least worst alternative.
=== added file 'android/ifaddrs.c'
--- android/ifaddrs.c 1970-01-01 00:00:00 +0000
+++ android/ifaddrs.c 2013-10-18 07:49:35 +0000
@@ -0,0 +1,600 @@
+/*
+Copyright (c) 2013, Kenneth MacKay
+All rights reserved.
Where does this come from? It does not follow coding style, has tabs and
trailing whitespace and seems to be from the times when checking your
mallocs was not yet fashionable.
https://github.com/kmackay/android-ifaddrs
It was included because it seemed to just work. I tried the one in aosp source
under external/dhdpcd also but it would have required borrowing quite a lot of
files from all around the aosp source and I have not managed to get it working
yet.
I guess Juhani could trim this with the bzr hooks (and please also add a
pointer to the URL where this was copied from).
I believe it is possible, but very tedious, to test memory leaks with
valgrind on Android, so unless you can point out a specific problem, my
suggestion is to ignore your comment on memory leaks. The impact is also
constrained because any potential problems in this file will only affect
the Android platform.
--- doc/hipl_android_preparation_guide.txt 1970-01-01 00:00:00 +0000
+++ doc/hipl_android_preparation_guide.txt 2013-10-18 07:49:35 +0000
--- libcore/builder.c 2013-08-19 18:30:29 +0000
+++ libcore/builder.c 2013-10-18 07:49:35 +0000
@@ -125,22 +125,32 @@
static void convert_byte_order(void *content, unsigned count,
unsigned item_size, enum cbo_flags flag)
{
- HIP_ASSERT(item_size == sizeof(uint16_t) || item_size == sizeof(uint32_t));
+ HIP_ASSERT(item_size == sizeof(uint16_t) || item_size == sizeof(uint32_t));
-
- if (item_size == sizeof(uint16_t)) {
- uint16_t *p = content;
- for (unsigned i = 0; i < count; i++) {
- p[i] = f16(p[i]);
- }
+ if (sizeof(uint16_t) == item_size) {
+ uint16_t *p = content;
+ if (CBO_HTON == flag) { // 16 bit host to network
+ for (unsigned i = 0; i < count; i++) {
+ p[i] = htons(p[i]);
+ }
- } else {
- uint32_t *p = content;
- for (unsigned i = 0; i < count; i++) {
- p[i] = f32(p[i]);
- }
- }
+
+ } else {
+ uint32_t *p = content;
+ if (CBO_HTON == flag) { // 32 bit host to network
+ for (unsigned i = 0; i < count; i++) {
+ p[i] = htonl(p[i]);
+ }
Why mess up the coding style?
The old code used function pointers (f16 and f32). Android defines ntohl,
htonl, ntohs and htons as macros. Macros can not be referenced by function
pointers, so the code would not compile. Did not find a way to remove function
pointers that would look as nice as the old one.
please double check this through the bzr hooks (e.g. add a newline in
end of the file and try commit).
You seem to be missing the uncrustify hook.
please run all changed files through the hooks.
--- libhipl/init.c 2012-07-12 11:32:14 +0000
+++ libhipl/init.c 2013-10-18 07:49:35 +0000
@@ -164,7 +167,11 @@
*/
static void sig_chld(int signum)
{
+ #ifdef CONFIG_HIP_ANDROID
+ int status;
+ #else
union wait status;
+ #endif /* CONFIG_HIP_ANDROID */
int pid;
This is wrong; using the union is nonsense here.
The union is from trunk; supposed it was good there because it always was and
it did work under linux.
Android didn't accept it though.
please change this in the trunk (int should be enough), test and commit.
For testing, I guess killing hipd when it is in the foreground and then
in background suffices as a test case.
Other related posts:
