review: needs-fixing On Fri, Oct 18, 2013 at 07:50:39AM -0000, Juhani Toivonen wrote: > --- INSTALL 2013-08-12 07:55:51 +0000 > +++ INSTALL 2013-10-18 07:49:35 +0000 > @@ -93,6 +93,61 @@ > +4. Set the environment variables printed at the end of the script. > + They are also inserted in your .bashrc so they will be set automatically > + each time you start a new terminal session. Auto-inserting stuff into .bashrc is evil. > --- Makefile.am 2013-10-17 23:06:20 +0000 > +++ Makefile.am 2013-10-18 07:49:35 +0000 > @@ -23,7 +23,7 @@ > > ACLOCAL_AMFLAGS = -I m4 > > -HIPL_HEADER_LOCATIONS = hipd/*.h hipfw/*.h libcore/*.h libcore/*/*.h > libhipl/*.h modules/*/*/*.h test/*.h test/*/*.h test/*/*/*.h > +HIPL_HEADER_LOCATIONS = hipd/*.h hipfw/*.h libcore/*.h libcore/*/*.h > libhipl/*.h modules/*/*/*.h test/*.h test/*/*.h test/*/*/*.h android > android/linux This was previously sorted. > @@ -167,6 +166,14 @@ > > +if !HIP_ANDROID > +libcore_libcore_la_SOURCES += libcore/capability.c > +endif > + > +if HIP_ANDROID > +libcore_libcore_la_SOURCES += android/ifaddrs.c > +endif An "else" could simplify this. > --- 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 > +#include <stdint.h> > +#include <inttypes.h> You don't need inttypes.h. > +/* 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... > +#endif missing #endif comment > === 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. > --- configure.ac 2013-08-06 19:17:14 +0000 > +++ configure.ac 2013-10-18 07:49:35 +0000 > @@ -104,8 +104,6 @@ > # can be substituted into our Python scripts. > AC_SUBST(pythondir, $(eval echo $pythondir)) > > - > - > # Set default AM_CFLAGS for the complete project. > AC_SUBST(AM_CFLAGS, "-std=c99 -Wall -Wextra -Werror") > AC_SUBST(AM_CFLAGS, "$AM_CFLAGS -Wredundant-decls -Wdisabled-optimization") > @@ -119,9 +117,25 @@ > # Set the preprocessor flags for the entire project > AC_SUBST(AM_CPPFLAGS, "-D_POSIX_C_SOURCE=200112L -D_XOPEN_SOURCE=500") > > - > # configure options stray changes > +AC_ARG_ENABLE(android, > + AS_HELP_STRING([--enable-android], > + [Enable android (default is NO)]), > + [ac_cv_use_android=$enableval], > + [ac_cv_use_android=no]) > +AC_CACHE_CHECK([whether to have verbose android], > + [ac_cv_use_android], > + [ac_cv_use_android=no]) verbose? > --- 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? > === modified file 'libcore/builder.h' > --- libcore/builder.h 2012-07-13 13:16:17 +0000 > +++ libcore/builder.h 2013-10-18 07:49:35 +0000 > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2010-2012 Aalto University and RWTH Aachen University. > + * Copyright (c) 2010-2013 Aalto University and RWTH Aachen University. > * > * Permission is hereby granted, free of charge, to any person > * obtaining a copy of this software and associated documentation > @@ -41,6 +41,9 @@ > #include "icomm.h" > #include "state.h" > > +#ifdef CONFIG_HIP_ANDROID > +#include "android/android.h" > +#endif /* CONFIG_HIP_ANDROID */ > > /* Removed in 2.6.11 - why ? */ > extern struct hip_cert_spki_info hip_cert_spki_info; > > === modified file 'libcore/conf.c' > --- libcore/conf.c 2012-06-03 10:26:55 +0000 > +++ libcore/conf.c 2013-10-18 07:49:35 +0000 > @@ -2681,7 +2684,7 @@ > * in conf.h because type values are used as @c action_handler array > * index. Locations and order of these handlers are important. > */ > -static int (*action_handler[])(struct hip_common *, int action, > +static int(*action_handler[]) (struct hip_common *, int action, > const char *opt[], int optc, int send_only) = > { stray change > --- libcore/filemanip.c 2011-10-25 21:44:47 +0000 > +++ libcore/filemanip.c 2013-10-18 07:49:35 +0000 > @@ -107,7 +108,7 @@ > /* Don't close file descriptor because new started process is > * running. */ > HIP_IFEL(fd <= 0, -1, "Opening lock file failed.\n"); > - HIP_IFEL(lockf(fd, F_TLOCK, 0), -1, "Lock attempt failed.\n"); > + HIP_IFEL(flock(fd, LOCK_EX|LOCK_NB), -1, "Lock attempt failed.\n"); spaces around | You seem to be missing the uncrustify hook. > --- 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. > --- libhipl/netdev.c 2012-06-03 10:26:55 +0000 > +++ libhipl/netdev.c 2013-10-18 07:49:35 +0000 > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2010-2012 Aalto University and RWTH Aachen University. > + * Copyright (c) 2010-2013 Aalto University and RWTH Aachen University. > * > * Permission is hereby granted, free of charge, to any person > * obtaining a copy of this software and associated documentation > @@ -54,6 +54,10 @@ > #include <sys/ioctl.h> > #include <linux/rtnetlink.h> > > +#ifdef CONFIG_HIP_ANDROID > +#include <linux/in6.h> > +#endif /* CONFIG_HIP_ANDROID */ > + > #include "libcore/builder.h" > #include "libcore/common.h" > #include "libcore/conf.h" > @@ -343,7 +347,7 @@ > list_for_each_safe(tmp, t, addresses, c) { > n = list_entry(tmp); > > - if (IN6_IS_ADDR_V4MAPPED(hip_cast_sa_addr((struct sockaddr *) > &n->addr)) == mapped) { > + if (IN6_IS_ADDR_V4MAPPED((const struct in6_addr *) > hip_cast_sa_addr((struct sockaddr *) &n->addr)) == mapped) { Why do you need to add the #include? > --- tools/prepare_android_toolchain.sh 1970-01-01 00:00:00 +0000 > +++ tools/prepare_android_toolchain.sh 2013-10-18 07:49:35 +0000 > @@ -0,0 +1,152 @@ > +#!/bin/bash Is this really bash-specific? > +# These are for the script's internals > +NDK_ROOT=${TOOLCHAIN_INSTALL_FOLDER}/android-ndk-r9 > +ANDROID_TOOLCHAIN=${TOOLCHAIN_INSTALL_FOLDER}/toolchain > +ANDROID_SYSROOT=${ANDROID_TOOLCHAIN}/sysroot > + > +# Functions > +make_install_folder() > +{ > + if [ ! -d ${TOOLCHAIN_INSTALL_FOLDER} ]; then > + if ! mkdir -p ${TOOLCHAIN_INSTALL_FOLDER}; then > + echo "Install failed. Could not create folder for toolchain." > + exit tab Diego
Attachment:
signature.asc
Description: Digital signature
