Re: [PATCH 05/12] util/error: allow non-NUL-terminated err->src

[Markus Armbruster <armbru@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 5/27/25 15:42, Markus Armbruster wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> Rust makes the current file available as a statically-allocated string,
>>> but without a NUL terminator.  Allow this by storing an optional maximum
>>> length in the Error.
>>>
>>> Note that for portability I am not relying on fprintf's precision
>>> specifier not accessing memory beyond what will be printed.
>> Can you elaborate on the portability problem?  I figure ...
>> 
>>>   {
>>>       if (errp == &error_abort) {
>>> +        const char *src = err->src;
>>> +        if (err->src_len >= 0) {
>>> +            /* No need to free it, the program will abort very soon...  */
>>> +            src = g_strndup(err->src, err->src_len);
>>> +        }
>>>           fprintf(stderr, "Unexpected error in %s() at %s:%d:\n",
>>> -                err->func, err->src, err->line);
>>> +                err->func, src, err->line);
>> ... you're avoiding the simpler
>>             fprintf(stderr, "Unexpected error in %s() at %.*s:%d:\n",
>>                     err->func, err->src_len, err->src, err->line);
>> because of it.
>
> I couldn't find anything that says %s is allowed to not be NUL-terminated if a precision is given.  That is, whether something like this:
>
>    char foo[] = {'H', 'e', 'l', 'l', 'o'};
>    printf("%.5s\n", foo);
>
> is guaranteed to work.

From ISO/IEC 9899:1999 §7.19.6.1 "The fprintf function":

       [#8] The conversion specifiers and their meanings are:
       [...]
       s       If  no  l  length  modifier is present, the argument
               shall be a pointer to  the  initial  element  of  an
               array of character  type.237)  Characters  from  the
               array  are  written  up  to  (but not including) the
               terminating null character.   If  the  precision  is
               specified, no more than that many bytes are written.
-->            If the precision is not specified or is greater than
-->            the  size  of  the  array, the array shall contain a
-->            null character.

       ____________________

       237No  special provisions are made for multibyte characters.

This clearly implies that the string need not be null-terminated when a
suitable precision is specified.  Which it is here.

> This is opposed to:
>
> 1) strnlen (https://pubs.opengroup.org/onlinepubs/9699919799/functions/strnlen.html), which is guaranteed to examine no more than the number of bytes given by the second character;
>
> 2) strndup, for which I found at least a clarification at https://www.austingroupbugs.net/view.php?id=1397.
>
> 3) g_strndup, which guarantees that the allocated block is of length n+1 and padded with NULs (though in the case above there will be just one NUL anyway)
>
> And also, for strndup/g_strndup it would be quite asinine to implement it using some kind of min(strlen(s), n) but for printf the complexity is greater so you never know.  I erred on the side of caution because avoiding an allocation before an abort() isn't particularly interesting.

Keeping the code simple is always interesting, though :)