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

[Markus Armbruster <armbru@redhat.com>]

Markus Armbruster <armbru@redhat.com> writes:

> 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.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  include/qapi/error-internal.h | 9 ++++++++-
>>  util/error.c                  | 5 +++--
>>  2 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/qapi/error-internal.h b/include/qapi/error-internal.h
>> index d5c3904adec..1ec3ceb40f0 100644
>> --- a/include/qapi/error-internal.h
>> +++ b/include/qapi/error-internal.h
>> @@ -18,7 +18,14 @@ struct Error
>>  {
>>      char *msg;
>>      ErrorClass err_class;
>> -    const char *src, *func;
>> +    const char *func;
>> +
>> +    /*
>> +     * src might be NUL-terminated or not.  If it is, src_len is negative.
>> +     * If it is not, src_len is the length.
>> +     */
>
> I habitually prefix identifiers with @ in comments, like this:
>
>        /*
>         * @src might be NUL-terminated or not.  If it is, @src_len is
>         * negative.  If it is not, @src_len is the length.
>         */
>
> Can really help readability when identifiers are also common English
> words.  Not much of a difference here.  Still nice for consistency with
> error.h.
>
>> +    const char *src;
>> +    int src_len;
>>      int line;
>>      GString *hint;
>>  };
>> diff --git a/util/error.c b/util/error.c
>> index e5bcb7c0225..3449ecc0b92 100644
>> --- a/util/error.c
>> +++ b/util/error.c
>> @@ -24,8 +24,8 @@ Error *error_warn;
>>  static void error_handle(Error **errp, Error *err)
>>  {
>>      if (errp == &error_abort) {
>> -        fprintf(stderr, "Unexpected error in %s() at %s:%d:\n",
>> -                err->func, err->src, err->line);
>> +        fprintf(stderr, "Unexpected error in %s() at %.*s:%d:\n",
>> +                err->func, err->src_len, err->src, err->line);
>>          error_report("%s", error_get_pretty(err));
>>          if (err->hint) {
>>              error_printf("%s", err->hint->str);
>> @@ -67,6 +67,7 @@ static void error_setv(Error **errp,
>>          g_free(msg);
>>      }
>>      err->err_class = err_class;
>> +    err->src_len = -1;
>>      err->src = src;
>>      err->line = line;
>>      err->func = func;

Almost forgot:

Reviewed-by: Markus Armbruster <armbru@redhat.com>