[Toybox] [PATCH] Silence a couple of warnings.

[Rob Landley]

> Personally I'd have made xstrdup take const char* like strdup(3), but
> I'm assuming that patch would be rejected...

Applied that bit. (The android stuff's yours, patch it how you like.)

As for the rest of the codebase, if you start adding "const" to stuff
you wind up doing regular patches adjusting "const" to silence warnings,
the only winning move is not to play.

I've still never seen const result in the optimizer doing a better job:
it doesn't put stuff in rodata and string constants already _are_. And
if you say "oh, it's not to produce better code, it's just a warning to
bad programmers who can't keep track of allocation context and yet
somehow people still want to use their C code" the fact it doesn't
auto-annotate read-only string constants with "const" because you'd be
assigning them to non-const char pointers all the time and everything
would be warnings means there's a pretty darn fundamental design flaw in
this entire idea, isn't there?

Maybe gcc 6 will start using it to optimize, but isn't the point of the
--whole-tree stuff that it can figure _out_ object lifetimes and where
data is and isn't modified? If a better optimizer needs manual hints the
current one isn't using, define "better". And trying to predict the
future is infrastructure in search of a user again.

Not a fan of const.

On 02/03/2017 06:17 PM, enh wrote:
>    // It's fine if this fails (generally because we're not root), but gcc no
> -  // longer lets a (void) typecast silence the "unused result" warning, so...
> -  if (fchown(fd, statbuf.st_uid, statbuf.st_gid));
> +  // longer lets a (void) typecast silence the "unused result" warning, and
> +  // clang requires the ; be on a new line for obviousness, so...
> +  if (fchown(fd, statbuf.st_uid, statbuf.st_gid))
> +    ;

So a line that exists in the first place to silence a dubious gcc
warning (which is wrong in this instance) needs to be modified to
silence a dubious clang warning (which is wrong in this instance), and
clang thought "significant whitespace" was a good solution to the
problem. (I blame C++.)

Does putting "fd = fd" before the ; fix the problem, or does it warn
about "statement obviously put there to be optimized away"? The NDK I've
got is still missing some stuff... Hmmm, lemme see if I can build just
this one file with it though:

> $ /opt/android/x86_64/bin/clang -c lib/lib.c -I .
> In file included from lib/lib.c:6:
> In file included from ./toys.h:70:
> ./lib/lib.h:133:42: error: expected ')'
> pid_t xpopen(char **argv, int *pipe, int stdout);
>                                          ^
> /opt/android/x86_64/bin/../sysroot/usr/include/stdio.h:83:17: note: expanded
>       from macro 'stdout'
> #define stdout (&__sF[1])

Hmmm... Huh,
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stdio.h.html
_does_ say they're supposed to be macros. Ok, my bad, I'll fix it.

(Um... how did that ever work for you? You did build this with
clang+bionic in AOSP, right?)

> lib/lib.c:1039:49: warning: adding 'int' to a string does not append to the
>       string [-Wstring-plus-int]
>   for (i=0; i<16; i++) out+=sprintf(out, "-%02x"+!(0x550&(1<<i)), uuid[i]);
>                                          ~~~~~~~^~~~~~~~~~~~~~~~

Um, yes? Because that's now how C works, and we're programming in C?
Why does it have a warning about the kind of mistake you shouldn't make
twice once you understand how the language works?

This is -Wtraining-wheels, why is it on by default? Ah the full toybox
build already has an -fwhatiswrongwithyou to suppress that.

> lib/lib.c:1133:11: warning: implicit declaration of function 'getgrgid_r' is
>       invalid in C99 [-Wimplicit-function-declaration]
>   errno = getgrgid_r(gid, &list->gr, sizeof(*list)+(char *)list,

According to
http://pubs.opengroup.org/onlinepubs/9699919799/functions/getgrgid_r.html
you include grp.h to get that, which toys.h is doing?

Right, what I _didn't_ see is a warning about the fd = fd; so I'll
assume that's an acceptable fix that doesn't add a dangling ; on its own
line deviating from the rest of the code style. (Significant whitespace.
Sigh.)

Rob

P.S. I want some sort of (i_know) typecast to make the compiler
consistently shut up about "assignment in an if statement" and so on.
The "may be used uninitialized, but isn't" nonsense even had its own
linux kernel macros at one point, but it's hard to grep for
"int i = i;" after the fact to see if the broken compiler still needs
that to stop warning about something you sat down and thought through
and no, it can't happen. There are no UNIFORM workarounds for this
nonsense that you can go through and clean out later...

P.P.S. I'd probably accept a patch upgrading the error handling, but I
refuse to be coerced into it by compilers objecting to valid C in
different ways every 3 months.