Re: [PATCH v1 4/7] ld.texi: Add ASCII to info file

[Ulf Samuelsson <binutils@emagii.com>]

[-- Attachment #1: Type: text/plain, Size: 3154 bytes --]


Den 2024-07-08 kl. 08:18, skrev Jan Beulich:
> On 05.07.2024 17:38, Ulf Samuelsson wrote:
>> Den 2024-07-02 kl. 10:12, skrev Jan Beulich:
>>> First: Please don't drop the mailing list from Cc. Second: Please don't
>>> top-post.
>>>
>>> On 02.07.2024 00:21, Ulf Samuelsson wrote:
>>>> I submitted the code in this patch series more than a year ago, and it got accepted.
>>>> Then a lot was reverted since I had not assigned the copyright to FSF.
>>>> For unknown reasons, my ASCIZ patches remained, but the ASCII stuff was reverted.
>>>> The reason for the naming is to be similar to how I remember naming in some assemblers where ASCIZ is zero terminated and ASCII is not zero terminated.
>>>>
>>>> If the string is too long, so it does not fit, it is really an error.
>>>> Adding a zero when truncating is a little less intrusive and may reduce runtime errors.
>>>> ASCII tells people what it is meant to be doing. ASCIIZ will make a lot of people wonder what the difference is between ASCIZ and ASCIIZ. It will also be misleading, because it does not add a zero if the string fits.
>>> Then did I read the code wrong? I was left with the impression that you'll
>>> truncate at size-1, to ensure zero termination. Plus as you say in this
>>> doc, size being 0 results in ASCIZ behavior, i.e. zero-terminates too.
>>                "          111111"
>>                "0123456789012345"
>>
>>     ASCII (16) "This is a string"
>>
>>     results in 'size' == 16. Since the string is 16 characters, 'len'
>> will be 16 as well.
> And hence
>
>    else if (len >= size)
>      {
>        len = size - 1;
>
> would reduce len down to 15 afaics, such that ...
>
Given *ASCII 16, "This is 16 bytes*"  size == 16 and strlen("This is 16 
bytes") == 16,
the linker should add a 16 byte area without a terminating NUL,

If the string is shorter, then the linker should pad with NUL.

If the string is longer, then the linker should truncate the string 
adding a NUL
at the end, still fitting in the area. A warning should be issued.

You could issue an error, but a warning is less intrusive.

Checked again and it needs to be changed from

   else if (len >= size)

to

   else if (len > size)

then the logic hopefully works out.


>>     This loop will be run 16 times, and i will be 16 at the exit of
>>
>>     for (i = 0; i < len; i++)
>>       lang_add_data (BYTE, exp_intop (string[i]));
>>
>>     Since i == 16, and size == 16, the following loop will not be run
>> even once.
>>
>>     while (i++ < size)
>>       lang_add_data (BYTE, exp_intop ('\0'));
>>
>>     so no '\0' will be added.
> ... a '\0' will be added here? Otherwise, if I continue to overlook
> something, what I can say is that ...
>
>> ===============================
>>
>>     ASCII (16) "This is a string."
>>
>>     size = 16, len = 17.  len > size, solen is set to 16-1 = 15.
>>
>>     copy the first 15 characters in the first loop, add a single '\0' in
>> the second loop.
> ... this would be pretty odd to me: Why would a nul be inserted in
> this case, but not in the one further up?
>
> Jan

-- 
Best Regards
Ulf Samuelsson