On 10/30/2017 01:34 PM, Vladimir Sementsov-Ogievskiy wrote:
> 27.10.2017 13:45, Eric Blake wrote:
>> Commenting these two lines is enough to avoid the change to 083.out
>> in 12/12.  That is evidence that we may want these two lines to be
>> trace points rather than error messages; or maybe we really do like
>> the extra verbosity in the case of an unexpected communication break.
>>
>> This patch does not meet coding guidelines, and I'm not proud enough
>> of it to give S-o-b, but I'm posting it for conversation.
>>

>>       if (ret < 0) {
>> -        error_report_err(local_err);
>> +        //error_report_err(local_err);
>>       }
>>       return ret;
>>   }
> 
> commenting is not ok, you should call error_free instead.

Indeed, and that's why I marked this RFC and used the wrong style
comment, to make it obvious that if we aren't going to print an error
message, there are better ways to go about doing that which don't leak
memory.  The question is whether the output is useful, or whether it
should be only via a trace (as it is, we aren't yet tracing anything in
block/nbd-client.c), or whether it is always just extra noise (in which
case passing the errp parameter around isn't helpful).


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to