[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-2.11] nbd/client: Use error_prepend() correc

From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH for-2.11] nbd/client: Use error_prepend() correctly
Date: Mon, 13 Nov 2017 18:10:37 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)

Eric Blake <address@hidden> writes:

> [adding Markus as error maintainer]
> On 11/13/2017 09:24 AM, Eric Blake wrote:
>> When using error prepend(), it is necessary to end with a space
>> in the format string; otherwise, messages come out incorrectly,
>> such as when connecting to a socket that hangs up immediately:
>> can't open device nbd://localhost:10809/: Failed to read dataUnexpected 
>> end-of-file before all bytes were read
>> Originally botched in commit e44ed99d, then several more instances
>> added in the meantime.
>> CC: address@hidden
>> Signed-off-by: Eric Blake <address@hidden>
>> ---
>>  nbd/client.c | 50 ++++++++++++++++++++++++++------------------------
>>  1 file changed, 26 insertions(+), 24 deletions(-)
>> diff --git a/nbd/client.c b/nbd/client.c
>> index 1880103d2a..4e15fc484d 100644
>> --- a/nbd/client.c
>> +++ b/nbd/client.c
>> @@ -79,12 +79,12 @@ static int nbd_send_option_request(QIOChannel *ioc, 
>> uint32_t opt,
>>      stl_be_p(&req.length, len);
>>      if (nbd_write(ioc, &req, sizeof(req), errp) < 0) {
>> -        error_prepend(errp, "Failed to send option request header");
>> +        error_prepend(errp, "Failed to send option request header: ");
> A quick grep of the tree noticed that most (all?) error_prepend()
> callers use trailing ": " in their format string.  Should we refactor
> that to be done automatically by error_prepend() itself, rather than at
> every callsite?

error_prepend() becomes less general, but perhaps a bit harder to

When I created it, I considered having it add ": ".  I rejected that,
because adding it in the caller is not much of a burden, and I assumed
that even the most basic testing would catch mistakes.

Turns out we can't be bothered to test new errors even once.

Missing colons in error messages is the least of my worries about
untested error paths.

I'd prefer to leave error_prepend() as it is.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]