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

Reply via email to