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
signature.asc
Description: OpenPGP digital signature