>> - printf(usage_array[i].aname);
>> + printf("%s", usage_array[i].aname);
> This can be replaced with fputs().
gcc does this automatically, see below.
>> static void xputenv(char *str)
>> {
>> if (putenv(str))
>> - bb_error_msg_and_die(bb_msg_memory_exhausted);
>> + bb_error_msg_and_die("%s", bb_msg_memory_exhausted);
> And this, and many other similar warnings, are just silly.
Worse: these changes are *harmful*.
It's ugly, and we really should not rely on that, and Manuel cannot be
blamed for not knowing it, but gcc performs a lot of magic around printf().
In this case, the bit of magic that interests us is that
printf(CONSTANT)
gets automagically replaced with
fputs(CONSTANT, stdout)
(You can test it by dynamically linking a minimal test program using
printf to write a constant string, and objdumping the resulting
binary. You will see a reference to fputs and no reference to printf.)
So, since bb_msg_memory_exhausted is constant, the printf part of
bb_error_msg_and_die(bb_msg_memory_exhausted);
is correctly optimized into
fputs(bb_msg_memory_exhausted, stdout);
whereas
bb_error_msg_and_die("%s", bb_msg_memory_exhausted);
uses the full printf() call - which behaves correctly, but is
suboptimal.
> Yes, gcc can't verify the format string, but it is declared
> as constant for a reason. These changes (all around bb_msg_*)
> are just adding code without fixing anything.
They fix the spurious warnings, but prevent fputs() optimizations
from being performed.
> In my opinion,
> gcc should be fixed instead, to at least treat const char[]
> fmt string as non-dangerous.
Exactly. Those gcc warnings are silly, because gcc will optimize away
the printf() call anyway.
--
Laurent
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
