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
Attachment:
pgpr9fzI0Wc7q.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list