On 18/04/2023 13:22, Eric Blake wrote:
Exploit the value of the flag for -n to reduce the size of
readlink_main() (regardless of the value of
CONFIG_FEATURE_READLINK_FOLLOW) on x86_64.
function old new delta
readlink_main 121 117 -4
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v4: Actually fix typo that killed the previous version by passing char
instead of char* to printf("%s")
---
coreutils/readlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/coreutils/readlink.c b/coreutils/readlink.c
index 0a9aa957e..8c4113cee 100644
--- a/coreutils/readlink.c
+++ b/coreutils/readlink.c
@@ -88,7 +88,7 @@ int readlink_main(int argc UNUSED_PARAM, char **argv)
if (!buf)
return EXIT_FAILURE;
- printf((opt & 1) ? "%s" : "%s\n", buf);
+ printf("%s%s", buf, "\n" + (opt & 1));
free(buf);
fflush_stdout_and_exit_SUCCESS();
base-commit: d2b81b3dc2b31d32e1060d3ea8bd998d30a37d8a
This results in another instance of a warning that is meant to be
suppressed, but is not suppressed, when building with CC=clang-16 (or
any other name for clang other than exactly "clang"):
coreutils/readlink.c:91:27: warning: adding 'unsigned int' to a string
does not append to the string [-Wstring-plus-int]
printf("%s%s", buf, "\n" + (opt & 1));
~~~~~^~~~~~~~~~~
coreutils/readlink.c:91:27: note: use array indexing to silence this warning
printf("%s%s", buf, "\n" + (opt & 1));
^
& [ ]
This warning is meant to be suppressed: from Makefile.flags:
# clang-9 does not like "str" + N and "if (CONFIG_ITEM && cond)" constructs
ifeq ($(CC),clang)
CFLAGS += $(call cc-option,-Wno-string-plus-int
-Wno-constant-logical-operand)
endif
However, as has been reported before, this check does not handle things
like CC=clang-16.
I am not sure what busybox's intent is here. If busybox decides that
this warning is useless, it would be nice if it got suppressed reliably.
Otherwise, if the suppression was meant to be temporary to allow code to
be adapted (in which case it is more than understandable that the
suppression is left imperfect: it is not worth extra effort), it would
be nice to write new code in the style they recommend to suppress the
warning, as &"\n"[opt & 1].
The rationale for the warning is that a novice user might be confused by
the fact that the + operator is overloaded in C++ to concatenate
strings, and despite the fact that this is not C++, might expect it to
do the same here, whereas &"\n"[opt & 1] indicates clear intent on the
programmer's part. Whether busybox agrees with this rationale is up to
them, but the warnings are unfortunate either way, just for different
reasons.
Cheers,
Harald van Dijk
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
