[libvirt] [PATCH v2 glib] gobject-stream: fix issue found by coverity

The coverity server found issue in gvir_stream_close function that we ignore return values of g_input_stream_close and g_output_stream_close, but we also ignore the error message and we assume that it's closed without error. Now we will check return values and also propagate the error message to the upper layers. We should try to close both streams even if closing the first one will fails. We can propagate only one error message. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- libvirt-gobject/libvirt-gobject-stream.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-stream.c b/libvirt-gobject/libvirt-gobject-stream.c index 1572022..46dbd9a 100644 --- a/libvirt-gobject/libvirt-gobject-stream.c +++ b/libvirt-gobject/libvirt-gobject-stream.c @@ -102,17 +102,31 @@ static GOutputStream* gvir_stream_get_output_stream(GIOStream *io_stream) static gboolean gvir_stream_close(GIOStream *io_stream, GCancellable *cancellable, - G_GNUC_UNUSED GError **error) + GError **error) { GVirStream *self = GVIR_STREAM(io_stream); + GError *local_error = NULL; + gboolean i_ret = TRUE, o_ret = TRUE; + + g_return_val_if_fail(error == NULL || *error == NULL, FALSE); if (self->priv->input_stream) - g_input_stream_close(self->priv->input_stream, cancellable, NULL); + i_ret = g_input_stream_close(self->priv->input_stream, cancellable, &local_error); + + if (local_error) + g_propagate_error(error, local_error); if (self->priv->output_stream) - g_output_stream_close(self->priv->output_stream, cancellable, NULL); + o_ret = g_output_stream_close(self->priv->output_stream, cancellable, &local_error); + + if (local_error) { + if (i_ret) + g_propagate_error(error, local_error); + else + g_error_free(local_error); + } - return TRUE; /* FIXME: really close the stream? */ + return (i_ret && o_ret); } -- 1.8.3.1

On Thu, Feb 20, 2014 at 04:58:21PM +0100, Pavel Hrdina wrote:
The coverity server found issue in gvir_stream_close function that we ignore return values of g_input_stream_close and g_output_stream_close, but we also ignore the error message and we assume that it's closed without error.
Now we will check return values and also propagate the error message to the upper layers. We should try to close both streams even if closing the first one will fails. We can propagate only one error message.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- libvirt-gobject/libvirt-gobject-stream.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 21.2.2014 12:24, Daniel P. Berrange wrote:
On Thu, Feb 20, 2014 at 04:58:21PM +0100, Pavel Hrdina wrote:
The coverity server found issue in gvir_stream_close function that we ignore return values of g_input_stream_close and g_output_stream_close, but we also ignore the error message and we assume that it's closed without error.
Now we will check return values and also propagate the error message to the upper layers. We should try to close both streams even if closing the first one will fails. We can propagate only one error message.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- libvirt-gobject/libvirt-gobject-stream.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
ACK
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Thanks, pushed. Pavel
participants (2)
-
Daniel P. Berrange
-
Pavel Hrdina