[Toybox] Android support

enh enh at google.com
Fri Nov 21 15:40:47 PST 2014


On Fri, Nov 21, 2014 at 4:54 AM, Rob Landley <rob at landley.net> wrote:
> On 11/21/14 00:42, enh wrote:
>> On Thu, Nov 20, 2014 at 9:24 PM, Rob Landley <rob at landley.net> wrote:
>>> On 11/20/14 12:33, enh wrote:
>>>> i notice you just added sethostname to portability.c which causes this
>>>> warning on Android:
>>>>
>>>> external/toybox/lib/portability.c:70:3: warning: implicit declaration
>>>> of function 'syscall' [-Wimplicit-function-declaration]
>>>>    return syscall(__NR_sethostname, name, len);
>>>>
>>>> the correct #include would have been #include <sys/syscall.h>, not
>>>> <asm/unistd.h>. you really shouldn't ever #include an asm/ file.
>>>
>>> Agreed.
>>>
>>> I merged the contribution mostly as-is because I don't have an android
>>> build environment set up. (I need to reinstall a system to get an
>>> environment to set up AOSP in, and the release deadline was 4 days
>>> overdue...)
>>>
>>>> i'm also curious about what your target is. AOSP master has
>>>> sethostname, so you don't need any of this. (but you do need some
>>>> other changes.)
>>>
>>> My target is making these guys happy:
>>>
>>> https://code.google.com/p/android/issues/detail?id=76861
>>>
>>> That said, I see they posted their own diff to that thread since the
>>> last time I looked at it, so I'll see about merging that instead.
>>
>> ah. that makes things easy then --- i'm the google guy on that bug :-)
>>
>> i'm happy just building toybox as part of the platform, and there --
>> to repeat what i said on the bug for the new audience on this list --
>> the only things we're missing right now are <pty.h>, <shadow.h>, and
>> <utmpx.h>.
>>
>> here's the patch i committed this morning:
>> https://android-review.googlesource.com/#/c/115676/1/lib/portability.h
>
> Checking in the .config file makes a certain amount of sense for your
> build environment, but why are you checking in generated/* files?
> They're rebuilt from scratch each time. (You might want to add the whole
> "generated" directory to your .gitignore instead of specific files.)

not by us; although your Makefile is checked in, the Android.mk is the
only makefile that actually gets used. we could change things to
generate the generated files into our build outputs directory and then
pull the files in from there, but that's a lot more work and -- unless
i'm missing something -- isn't currently necessary.

> Do you try to get Android.mk files upstream into projects, or do you
> prefer to maintain them locally? (I could add a scripts/android
> directory with your .config and Android.mk in it, but I don't know if
> that would help.)

if an upstream project is actively working on Android as a target it
can make sense, but in a case like this it probably just means more
work for us and more work for you.

>> for a real patch you'd probably want to stop building the getdelim and
>> getline implementations too. i can send one to the list if that'd
>> help.
>
> Yes please. I'm not quite sure what bionic provides here. I've
> encountered three states:
>
> 1) libc provides getdelim/getline and knows it.

that's bionic's current state. (but wasn't prior to L.)

> 2) libc provides getdelim/getline but the headers won't reliably give
> prototypes for them.
>
> 3) libc doesn't provide getdelim/getline, we need to provide our own.
>
> The existing patch seems to put bionic in #2 above?

no, that was just me minimizing my diff from upstream until i got
chance to talk to you. i've attached a patch against your current ToT
that builds in AOSP master.

>>> I'll try to get the better android patch applied this weekend. (The
>>> annotation of individual commands that won't work without utmpx and such
>>> in the old patch is good, so I should merge the two...)
>>
>> yes, the annotations are a nice solution. i wish i'd had that a week ago :-)
>
> I've merged your portability.h change, let me know if there's more.
>
>> i do plan on implementing <pty.h>, hopefully before the next release,
>> but i didn't want to hold up getting started on using toybox until
>> that's done.
>
> Have you seen musl-libc.org? It's a very nice MIT-licensed libc (written
> from scratch in the past ~4 years) that you might be able to lift code
> like that from.

we've looked at some pieces of that, but it's seemed pretty buggy and
definitely lacking in tests. at the moment most of our portable stuff
comes from OpenBSD and most of our Linux-specific stuff is stuck with
annoying ABI constraints. plus we have a few extra architectures we
need to worry about.

that said, i do suspect our most lasting contribution to the c library
state of the art will be our unit tests...

>>> Thank you,
>>>
>>> Rob
>>
-------------- next part --------------
diff --git a/lib/portability.c b/lib/portability.c
index 17efc91..910b1ea 100644
--- a/lib/portability.c
+++ b/lib/portability.c
@@ -5,11 +5,8 @@
  */
 
 #include "toys.h"
-#if defined(__ANDROID__)
-#include <sys/syscall.h>
-#endif
 
-#if defined(__APPLE__) || defined(__ANDROID__)
+#if defined(__APPLE__)
 ssize_t getdelim(char **linep, size_t *np, int delim, FILE *stream)
 {
   int ch;
@@ -62,16 +59,7 @@ ssize_t getline(char **linep, size_t *np, FILE *stream)
 {
   return getdelim(linep, np, '\n', stream);
 }
-#endif
-
-#if defined(__ANDROID__)
-int sethostname(const char *name, size_t len)
-{
-  return syscall(__NR_sethostname, name, len);
-}
-#endif
 
-#if defined(__APPLE__)
 extern char **environ;
 
 int clearenv(void)
diff --git a/lib/portability.h b/lib/portability.h
index 548c128..2f2421d 100644
--- a/lib/portability.h
+++ b/lib/portability.h
@@ -187,9 +187,6 @@ ssize_t getline(char **lineptr, size_t *n, FILE *stream);
 #include <sys/swap.h>
 
 // Android is missing some headers and functions
-#if defined(__ANDROID__)
-int sethostname(const char *name, size_t len);
-#endif
 // "generated/config.h" is included first
 #if CFG_TOYBOX_SHADOW
 #include <shadow.h>
diff --git a/toys/other/pivot_root.c b/toys/other/pivot_root.c
index 3e4beac..9a1f56c 100644
--- a/toys/other/pivot_root.c
+++ b/toys/other/pivot_root.c
@@ -22,7 +22,8 @@ config PIVOT_ROOT
 #define FOR_pivot_root
 #include "toys.h"
 
-#include <linux/unistd.h>
+#include <sys/syscall.h>
+#include <unistd.h>
 
 void pivot_root_main(void)
 {


More information about the Toybox mailing list