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

Pavel Hrdina phrdina at redhat.com
Thu Feb 20 14:57:00 UTC 2014


On 20.2.2014 15:07, Christophe Fergeau wrote:
> Hey,
>
> On Thu, Feb 20, 2014 at 01:49:31PM +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_outpu_stream_close, but we also ignore the error message and we
>
> missing 't' in g_output_stream_close

Oh, thanks, I'll fix that.
>
>> assume that it's closed without error.
>>
>> Now we will check return values and also propagate the error message
>> to the upper layers.
>>
>> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
>> ---
>>   libvirt-gobject/libvirt-gobject-stream.c | 24 ++++++++++++++++++++----
>>   1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/libvirt-gobject/libvirt-gobject-stream.c b/libvirt-gobject/libvirt-gobject-stream.c
>> index 1572022..66a12ab 100644
>> --- a/libvirt-gobject/libvirt-gobject-stream.c
>> +++ b/libvirt-gobject/libvirt-gobject-stream.c
>> @@ -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.

Pavel




More information about the libvir-list mailing list