[Toybox] [PATCH][v2] probe for android oddities

Rob Landley rob at landley.net
Wed Nov 19 15:04:54 PST 2014


On 10/31/14 12:49, Isaac Dunham wrote:
> probe for getspnam(), forkpty(), utmpx, replace sethostname()

Finally taking a close look at this. (I never did get an AOSP build
environment set up, but I'm out of time for the release...)

+#if defined(CFG_TOYBOX_SHADOW) && CFG_TOYBOX_SHADOW

The CFG_ symbols should be unconditionally #defined to a value
(generally either 0 or 1) so you can use them in if() statements and let
dead code elimination cut out calls to unused functions without
sprinkling #ifdefs all over the code (and thus making your variable
declarations and number of curly brackets potentially not match up
depending on your configuration selection, leading to config-dependant
build breaks.)

I.E. if you're actually doing "#if CFG_BLAH", you shouldn't need the
defined().

Also:

+  USE_TOYBOX_UTMPX(struct utmpx *entry;)
+  USE_TOYBOX_UTMPX(int users = 0;)

This is the one-line version of putting an #ifdef around those things.
If you want to make the command depend upon a guard symbol, fine, but
the USE(variable declaration;) can cause config dependent build breaks
if something uses the variable later on and only ever tests on a system
with the relevant config thing enabled.

> Android is missing all of these; we need to probe for some so we have
> a config symbol to depend on.

Is there a reason for adding #include <termios.h>? It seems unrelated...?

> sethostname() is easily replaced.
> We got termios.h via pty.h; now it's not included in configure-step tools,
> so we need termios.h to generate globals.

Ah, there's the explanation.

> --
> Compile tested on Alpine Linux; I don't have the NDK.

Neither do I, which is why I held off. (That and musl can be used on
android, although I think bionic's got some magic passthrough bits to
talk to their kernel extensions and android's version of the windows
registry and so on, so sometimes you can't avoid building against it...)

> Android currently has swapon/swapoff, so there's no need to replace them.

Has them in libc, or has them in toolbox?

In the case of swapon, does it implement -d? That's on my todo list. I
should also do swapon -a and swapoff -a...

> Thanks,
> Isaac Dunham

I did a second fixup patch on top of it. (The PTY test was always
failing because you haven't #included the header that defines NULL, and
when it wasn't enabled netcat gave a warning about an undefined function
name so I prototyped a stub...)

Ok, it'sin. No idea if it _works_, but it doesn't seem to be screwing up
normal builds...

Rob

 1416438294.0


More information about the Toybox mailing list