On Mon, Apr 17, 2023 at 11:21:21PM +0200, tito wrote: > On Mon, 17 Apr 2023 14:15:40 -0500 > Eric Blake <ebl...@redhat.com> wrote: > > > Exploit the value of the flag for -n to reduce the size of > > readlink_main() (shown here with CONFIG_FEATURE_READLINK_FOLLOW off) > > on x86_64. > > > > function old new delta > > readlink_main 121 118 -3
> > - printf((opt & 1) ? "%s" : "%s\n", buf); > > + printf("%s%s", buf, "\n"[opt & 1]); > > free(buf); > > > > fflush_stdout_and_exit_SUCCESS(); > > > > base-commit: d2b81b3dc2b31d32e1060d3ea8bd998d30a37d8a > > Hi, > I was just curious as with my limited C skills I didn't understand how > "\n"[opt & 1] works, If it makes it easier, you could use the more verbose: const char *tail = "\n"; printf("%s%s", buf, tail[opt & 1]); but I didn't see a reason to declare a one-shot variable. In C, both 'pointer[integer]' and 'integer[pointer]' are shorthands for the same thing (you usually write the pointer outside of the [], but its a nice obfuscation trick to know that you could equally write (opt & 1)["\n"] for the same results). Either way, those shorthands are turned by the compiler into *(pointer+offset), that is, treat the given pointer as an array and dereference the integer offset into that array (where the integer is scaled by the size of the array element as determined by the underlying type of the pointer). The other thing to know is that string literals are pointers. That is, "\n" is a 'const char *' pointing to at least the two bytes '\n' and '\0'. So [opt & 1] is the integer offset that says how far into that array to read. If opt&1 is 0 (-n was not passed), we want offset 0, effectively matching %s to the sub-array beginning {'\n','\0'} (in short form "\n") and printing the trailing newline. If opt&1 is 1 (-n was passed), we want offset 1, effectively matching %s to the sub-array beginning {'\0'} (in short form "") and printing nothing. > so I applied your patch and compiled it, > but it seems to me that something is wrong: > > Prepare a link for testing: > tito@devuan:~/Desktop/SourceCode/busybox_new$ ln -s busybox prova > > Busybox with your patch applied: > $ ./busybox readlink prova > Segmentation fault > $ ./busybox readlink -n prova > busybox(null)$ Eww - that wasn't happening for me; but I had CONFIG_REATURE_READLINK_FOLLOW turned off; when I re-enable that, I'm seeing the same crash. :( This patch specifically depends on -n mapping to bit 0 (value 1) (as otherwise, [opt & 1] dereferences out of bounds of the 2-element array referenced by "\n"). Beyond that, I'm not sure why things are behaving differently; maybe I'm misunderstanding how getopt32() works? > > Real readlink; > readlink prova > busybox > $ readlink -n prova > busybox$ > > Did you intend something like: > > printf("%s%c", buf, '\n'*!(opt & 1)); No, because that prints an actual NUL byte, which is incorrect. The point of -n is to print nothing (0 bytes output), not a NUL byte (1 byte output). > > I did not test if this reduces size. Doesn't matter if it reduces size if it is wrong. But I'm still trying to figure out why CONFIG_REATURE_READLINK_FOLLOW doesn't work with my patch? I'll have to figure out how to recompile with debugging symbols left intact to run it under gdb, to get a more informative stack trace than: #0 __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76 #1 0x00007ffff7d2e828 in __vfprintf_internal ( s=0x7ffff7ea3780 <_IO_2_1_stdout_>, format=0x4e09e0 "%s%s", ap=ap@entry=0x7fffffffda50, mode_flags=mode_flags@entry=0) at /usr/src/debug/glibc-2.36-9.fc37.x86_64/stdio-common/vfprintf-process-arg.c:397 #2 0x00007ffff7d228ff in __printf (format=<optimized out>) at printf.c:33 #3 0x0000000000499a42 in readlink_main () #4 0x0000000000000000 in ?? () -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org _______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox