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

Re: [libvirt] [PATCH v4 6/6] virStream*All: Report error if a callback fails




On 06/22/2017 08:30 AM, Michal Privoznik wrote:
> All of these four functions (virStreamRecvAll, virStreamSendAll,
> virStreamSparseRecvAll, virStreamSparseSendAll) take one or more
> callback that handle various aspects of streams. However, if any

(same as previous review)

s/callback/callback functions/

> of them fails no error is reported therefore caller does not know
> what went wrong.
> 
> At the same time, we silently presumed callbacks to set errno on
> failure. With this change we should document it explicitly as the
> error is not properly reported.
> 
> Signed-off-by: Michal Privoznik <mprivozn redhat com>
> ---
>  include/libvirt/libvirt-stream.h | 17 +++++++++++++-
>  src/libvirt-stream.c             | 50 +++++++++++++++++++++++++++++++++-------
>  2 files changed, 58 insertions(+), 9 deletions(-)
> 

I think you could move all those "errno = 0;" settings outside the "for
(;;) {" loop beginnings. Unless it's felt some called function would
change errno to something and return 0. They're not wrong where they
are, so it's your call.

You could also alter each of the new comments requesting the setting of
errno to something to indicate failure to set errno will cause libvirt
to default to using EIO.

Reviewed-by: John Ferlan <jferlan redhat com>

John


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