[PATCH v2] readlink: slight size optimization

[Harald van Dijk]

On 17/04/2023 22:21, tito wrote:
> On Mon, 17 Apr 2023 14:15:40 -0500
> Eric Blake <eblake at 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
>>
>> Signed-off-by: Eric Blake <eblake at redhat.com>
>>
>> ---
>>
>> v2: Add 'make bloatcheck' details
>> ---
>>   coreutils/readlink.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/coreutils/readlink.c b/coreutils/readlink.c
>> index 0a9aa957e..83c417e66 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
> 
> Hi,
> I was just curious as with my limited C skills I didn't understand how  "\n"[opt & 1] works,
> so I applied your patch and compiled it,
> but it seems to me that something is wrong:
> 
> Prepare a link for testing:
> tito at 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)$

Your testing is correct: the patch is wrong and cannot work.

> Real readlink;
> readlink prova
> busybox
> $ readlink -n prova
> busybox$
> 
> Did you intend something like:
> 
> printf("%s%c", buf, '\n'*!(opt & 1));

I am assuming it was intended to be

   printf("%s%s", buf, &"\n"[opt & 1]);

As you are trying to understand how this works: the string "\n" is a 
character array consisting of the characters '\n' and '\0', where '\0' 
acts as the string terminator. &"\n"[0] is a pointer to the first 
element of this array and will be printed as the character '\n'. 
&"\n"[1] is a pointer to the second element of this array, the null 
character, meaning it is a null string and printing it has no effect.

This is different from the approach you were taking: printing '\0' with 
%c results in the output of a null byte. You would not see this null 
byte if you visually inspect the output, but you would if you e.g. view 
the output with the 'od' utility.

Cheers,
Harald van Dijk

> I did not test if this reduces size.
> 
> Ciao,
> Tito