Post by Rob LandleyPost by enhPost by Rob LandleyI intend to finish and promote it this month, for what that's worth.
(I've got another 2 weeks...)
yeah, for now i've taken ps out of the Android .config. we didn't have
a symbolic link to it anyway, and although it's making progress, it's
not currently very likely that anyone would want to "toybox ps".
here's the current list of things from pending that get built into the
AOSP toybox binary. ones marked with * get a symbolic link too.
toys/pending/dd.c
toys/pending/expr.c *
toys/pending/hwclock.c *
toys/pending/more.c *
toys/pending/pgrep.c *
toys/pending/netstat.c *
toys/pending/route.c *
toys/pending/tar.c *
toys/pending/top.c
toys/pending/tr.c *
toys/pending/traceroute.c
those with links are either demonstrably better than what we had, or
things we just didn't have before. so although rob worries, it's not
like we're shipping all of pending and relying on it :-)
I'm cleaning up as fast as I can!
Speaking of which, here's a proposed getprop.c cleanup that I haven't
got a test environment for. The attached patch is infrastructure
changes, then the changed getprop.c file, and here's the notes from
---------
Proposed changes! (I'm not checking this in because I need your review
before doing that, I don't have your build environment or domain
expertise here.)
Instead of an #ifdef, let's add a "depends on TOYBOX_ON_ANDROID" so
you can only select the sucker when we're building on an android
system. (Meaning I can't test this, but I couldn't anyway because I
didn't have the header. I need to chop a build environment out of the
AOSP build...) [todo: teach "make change" about probed symbols.]
this breaks things for me. my goal is to have one .config file but
build for both the host and Android, so i can run tests on the host
(where i have stuff like awk).
Post by Rob LandleyGlobals go in GLOBALS(), this includes "properties" here.
Google says PROPERTY_VALUE_MAX is 92, sizeof(toybuf) is 4096, so let's
just use that.
Since properties is an array of pointers to 2 entry structures, just
put the structs inline in the array. Since the name is the first entry
and you can typecast a struct pointer to a pointer to its first entry
(C99 says so), we can use strcmp() directly in qsort, so that function
goes away.
except results are no longer sorted.
Fix getprop sorting.
Use qstrcmp instead of alphasort (which expects struct dirent arguments).
diff --git a/toys/android/getprop.c b/toys/android/getprop.c
index 400d80e..71289f0 100644
--- a/toys/android/getprop.c
+++ b/toys/android/getprop.c
@@ -41,7 +41,7 @@ void getprop_main(void)
size_t i;
if (property_list((void *)add_property, 0)) perror_exit("property_list");
- qsort(TT.nv, TT.size, 2*sizeof(char *), alphasort);
+ qsort(TT.nv, TT.size, 2*sizeof(char *), qstrcmp);
for (i = 0; i<TT.size; i++) printf("[%s]: [%s]\n",
TT.nv[i*2],TT.nv[1+i*2]);
if (CFG_TOYBOX_FREE) free(TT.nv);
}
Post by Rob Landley(Except we have to wrap it anyway to do an extra dereference. [See
GIANT_RANTY_FOOTNOTE at the bottom] So I added a qstrcmp() function to
lib/lib.c.)
Also the free() function is just free() the array at the end so that function
goes away.
Also in add_property we realloc() with 32 more slots each time size is
a multiple of 32, so we don't need a separate capacity counter. (Since 0
is an even multiple of 32 we alloc the initial data the same way.)
I'm assuming that property_list() will set errno on a failure, and thus used
perror_exit(). (This is a questionable assumption I can't test, hence the shout
out here.)
no, it doesn't. since you won't have applied the previous patch by the
time you get here...
Fix getprop sorting and error reporting.
Use qstrcmp instead of alphasort (which expects struct dirent arguments).
Don't use perror_exit because property_list doesn't set errno.
diff --git a/toys/android/getprop.c b/toys/android/getprop.c
index 400d80e..09bb0f0 100644
--- a/toys/android/getprop.c
+++ b/toys/android/getprop.c
@@ -40,8 +40,8 @@ void getprop_main(void)
} else {
size_t i;
- if (property_list((void *)add_property, 0)) perror_exit("property_list");
- qsort(TT.nv, TT.size, 2*sizeof(char *), alphasort);
+ if (property_list((void *)add_property, 0)) error_exit("property_list");
+ qsort(TT.nv, TT.size, 2*sizeof(char *), qstrcmp);
for (i = 0; i<TT.size; i++) printf("[%s]: [%s]\n",
TT.nv[i*2],TT.nv[1+i*2]);
if (CFG_TOYBOX_FREE) free(TT.nv);
}
Post by Rob LandleyAbout changing the xstrdup() prototype: Toybox doesn't use "const" because
I've never encountered a compiler that does anything intelligent with it
(what's it gonna do, put stack variables in a read-only segment?) and we
otherwise spend a huge amount of time getting it all to line up with no
http://landley.net/toybox/cleanup.html#advice
it helps catch careless mistakes. plus, here, it's weird to have a
wrapper around a function that takes const char* where the wrapper
takes the more restrictive (and misleading) char*.
Post by Rob Landley(I'm aware property_list() wants const in its prototype, and am hitting that
with (void *) which is ugly but still fewer typecasts than before.)
but you'd have zero typecasts if xstrdup didn't have an incorrect prototype!
Post by Rob LandleyAnd looking at it: we don't really need the struct at all because it's just
pairs of char * with alternating meanings, that's simple enough to just do
inline. (This is a "six of one, half dozen of another" thing.)
Query: why size_t here? Is more than 2 billion of them likely to come up?
(Left it alone, but...)
size_t is the type of an index/size. if you use something _other_ than
size_t, your reader has to stop and wonder whether something subtle is
going on. if you use size_t, no thinking required.
Post by Rob LandleyRob
qsort() really should have standard sort functions for things like
strcmp() on an array of pointers (or structs whose first member is a
pointer, which work the same way as far as qsort() is concerned). The
man page even has EXAMPLE CODE showing how to do this, and yet it's
not in libc and everybody needs to do their own gratuitous wrapper
function.
It _does_ say we can use alphasort(), which is wrong. That says it
wraps strcoll() which is equivalent to strcmp() in the "C" locale
(which we're in if we didn't feed TOYFLAG_LOCALE to the flags up top,
but I dunno if bionic actually implements that, even though it's in
C99. But that's not the wrong part, the wrong part is that when I
actually _try_ it I get incompatible pointer type because the man page
says it's alphasort(const void *a, const void *b) and the compiler is
saying "oh no, those arguments are struct dirent **"...
http://austingroupbugs.net/view.php?id=142
So posix-2008 is forcing people to use a typecast to get historical
behavior _in_the_name_of_type_safety_. That's just hilarious... (Ok,
looking deeper the Solaris guys broke this, then Jorg Schilling pushed
it upstream into posix, and it's up to _me_ to email Michael Kerrisk
to fix the man page _7_years_ later. And no, the solaris guys _really_
broke this so it doesn't work anymore even if you typecast. So the man
page needs fixing and the posix guys need somebody to mail them a box
of live stinging insects with a card that plays "seasons in the sun"
when you open it and doesn't stop when you close it again until the
battery runs down. Use a lithium battery.)
(anyone who cares has already moved on to C++ anyway. what the POSIX
folks do to qsort matters little to the people of 2015 passing lambdas
to std::sort or just using a sorted container. or, you know, Java or
Python.)
Post by Rob LandleyAnyway, I added a lib.c function for this. I was TEMPTED to add a
portability.c function and just declare posix's actions on this too
dumb to live, but somebody who shall remain nameless but used to
maintain cdrecord seems to ahve hijacked the posix committee into
doing stupid and I haven't got time to try to engage with the stupid
and move an elephant out of my way. So, lib.c function qstrcmp() and I
walk away from the keyboard for a bit.
you should have waited until you were using your new function before
walking away :-)
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.