On Thu, Feb 20, 2014 at 03:57:00PM +0100, Pavel Hrdina wrote:
On 20.2.2014 15:07, Christophe Fergeau wrote:
@@ -102,17 +102,33 @@ 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;
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) { + if (!*error) + g_propagate_error(error, local_error); + else + g_error_free(local_error); + }
g_propagate_error will be doing this if (!*error) check for you, so this can be written as if (local_error) g_propagate_error(error, local_error);
void g_propagate_error (GError **dest, GError *src); If dest is NULL, free src; otherwise, moves src into *dest.
Christophe
The g_propagate_error will free the src only if error == NULL but if the *error is not NULL it will print warning and do nothing with the src and therefore there could be memory leak. That's why I'm checking if *error is null and otherwise I'll free the local_error.
If error is NULL, this test will cause a NULL pointer dereference. Passing a GError ** pointing to a non-NULL GError is a programming error, so what is done in other places in libvirt-glib is to have a g_return_val_if_fail(error == NULL || *error == NULL, FALSE); at the beginning of the function to reject such invalid calls right away at runtime. If you do that, the first check and g_error_free() can be removed. If an error occurred there, and if the g_output_stream_close() call fails, we'll be in a situation when g_propagate_error() can try to overwrite an already set error. g_io_stream_close() says "On failure the first error that happened will be reported, but the close operation will finish as much as possible." so we need to do the g_output_stream_close() call even if the g_input_stream_close() call failed, but not try to set the error when this happened. Christophe