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

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



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 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


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