[Toybox] [PATCH] fix ps.c build

[Rob Landley]

On 05/02/2015 02:47 PM, enh wrote:
>> 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).

If you're building against an x86 version of bionic on the host, it'll
consider it android. If you _don't_ have that, it's a build break to
enable this stuff anyway...?

>      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);
>    }

Oops. Implemented a fix, checked in the library file, did not check in
the change to the file that triggered it. (Sigh. The git status/diff
stuff vs mercurial status/diff stuff are just different enough to
repeatedly throw me off here...)

>> 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.

Applied.

>> About 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
>> actual benefit. There's a cleanup.html entry to this effect:
>>   http://landley.net/toybox/cleanup.html#advice
> 
> it helps catch careless mistakes.

String constants being in the read-only data segment should help catch
careless mistakes. I've yet to see const do that. (It _can't_ for stack
data.)

 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*.

It's from me trying to be consistent within toybox, where nothing uses
const.

I broke down and put the const on qstrcmp() because every single use of
it would have to be typecast otherwise. (I've been tempted to switch the
dlist stuff to void * for the same reason.) But xstrcmp() shouldn't
throw warnings whether or not you feed it a const?

>> (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!

Or if property_list() didn't have a meaningless qualifier that exists
solely to be matched with other meaningless qualifiers.

(I experimented with "#define const" as the first include early in
toybox' history. It didn't help. I forget why.)

>> And 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.

But that logic the i in for loops should be size_t.

> 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.

For me it was the other way. I had to stop and think about whether there
was something subtle going on.

I like to know what types I'm actually using. This is why toys.optflags
is an int rather than a long: on 32 bit platforms you get 32 bits (thus
32 possible flags). On 64 bit platforms, you get: 32 bits. You don't
have something that works on 64 bits but _doesn't_ work on 32 bits.

But size_t varies with "long file support" (archaic but still possible
to forget to enable because the glibc default is still crazy:
https://sourceware.org/ml/libc-alpha/2014-03/msg00092.html ). This is
the _reason_ for size_t. Because it might _not_ be 64 bits.

The above "i in for loops" example has a built-in reason _not_ to use
it: manipulating 64 bit numbers on 32 bit systems is awkward so you
_don't_ want to have a 64 bit index counter in a performance critical
inner loop in that context (and you _really_ want to avoid 64 bit
divides there). That isn't the case here but also isn't something you
want to forget and do it wrong where it does matter. (I've seen people
complain that large file support slowed down their programs and it was
generally them pulling that sort of thing.)

This isn't a big enough deal for me to bother to change it, but the "no
thinking required" above? Not a compelling argument for me in this
instance. :)

>> Rob
>>
>> P.S. GIANT_RANTY_FOOTNOTE:
...
> (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.)

And when they collapse under the weight of their endlessly increasing
complexity, C will presumably still be here. :)

(The saving grace of ISO-WG14 and the Austin group is neither actually
does very much, and when they do they're mostly* documenting existing
practice. I'm still ignoring the 2011 C standard, because I can. I read
the summaries when it came out, and other than the new thread
synchronization stuff (NNPTL? NITL?) I think it was all fluff. The musl
guys do care, and I cheer them on from the sidelines with popcorn.)

(Don't get me started on Python. Honestly, python 3 is not the same
language as python 2. One obvious way to do it indeed. They have
_more_than_one_ conference panel at pycon about the lack of uptake of
their rewrite (ala https://lwn.net/Articles/640181/ and
https://lwn.net/Articles/640179/) six and a half years after its
release, and they don't think this is damaging the language? A c89
program will generally compile and run under a c99 compiler. Python 3
breaks python 2. This is a problem.)

* Yeah, I know.

>> Anyway, 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 :-)

I totally should have, yes. Should be fixed now...

Rob

 1430841222.0