On Mon, 17 Apr 2023 17:23:04 -0500 Eric Blake <[email protected]> wrote:
> On Mon, Apr 17, 2023 at 11:21:21PM +0200, tito wrote: > > On Mon, 17 Apr 2023 14:15:40 -0500 > > Eric Blake <[email protected]> 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. Thanks for the explanation, I see how it was intended to work. > > 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. :( Just for the record I've tested it with # CONFIG_FEATURE_READLINK_FOLLOW is not set > 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 ?? () > _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
