[Axl] [BUG] memory leaks

[Francis Brosnan Blazquez]

Hi Benoit,

Taking your patch, here is a check list for each possible bug reported
and its associated solution:

> Index: src/axl_doc.c
> ===================================================================
> --- src/axl_doc.c       (revision 3355)
> +++ src/axl_doc.c       (working copy)
> @@ -1077,12 +1077,12 @@
>          * decoder (only if defined handler found) */
>         if (detect_codification_func) {
>                 if (! detect_codification_func (stream,
> &doc->detected_encoding, detect_codification_data, error))
> -                       return NULL; 
> +                       goto fail; 

This is a memory leak since the detect_codification_func could fail and
hence, it is required to free axlStream ref. This is now fixed and
integrated into RT.

>         } /* end if */
>  
>         /* parse initial xml header */
>         if (!__axl_doc_parse_xml_header (stream, doc, error))
> -               return NULL;
> +               goto fail;

This is not a memory leak since every case that could cause the function
to fail and return the axlStream reference is deallocated.

>         /* signal that this document have processed its header */
>         doc->headerProcess = true;
> @@ -1090,7 +1090,7 @@
>         /* parse the rest of the document, setting as parent NULL
>          * because still no parent is found. */
>         if (!__axl_doc_parse_node (stream, doc, &node, &is_empty,
> error))
> -               return NULL;
> +               goto fail;

Same as previous. Every case that could cause a failure is protected
with an axl_stream_free.

>         /* if the node returned is not empty */
>         if (! is_empty) {
> @@ -1115,7 +1115,7 @@
>                         /* consume a possible comment and process
> instructions */
>                         if (axl_stream_peek (stream, "<?", 2) > 0 ||
> axl_stream_peek (stream, "<!--", 4) > 0) {
>                                 if (! axl_doc_consume_comments (doc,
> stream, error))
> -                                       return false;
> +                                       goto fail;

Same as previous: axl_doc_consume_comments have calls to axl_stream_free
on every failure case.
  
>                                 /* continue on the next index */
>                                 continue;
> @@ -1129,7 +1129,7 @@
>  
>                                 /* seems that a node is being closed
> */
>                                 if (! __axl_doc_parse_close_node
> (stream, doc, &node, error))
> -                                       return NULL;
> +                                       goto fail;

Same as previous.

>                                 /* because the xml node have been
>                                  * closed, make the parent to be the
> @@ -1164,8 +1164,7 @@
>                                 axl_stream_set_buffer_alloc (stream,
> NULL, NULL);
>                                 if (string == NULL) {
>                                         axl_error_new (-1, "Unable to
> get CDATA content. There was an error.", stream, error);
> -                                       axl_stream_free (stream);
> -                                       return NULL;
> +                                       goto fail;
>                                 }

In this case it is not required to goto to fail since calling to
axl_stream_free terminates the streams and objects associated
(axl_stream_link) in this case the document reference. 

>                                 /* nullify internal reference to the
> @@ -1189,7 +1188,7 @@
>  
>                                 /* seems that another node is being
> opened */
>                                 if (!__axl_doc_parse_node (stream,
> doc, &node, &is_empty, error))
> -                                       return NULL;
> +                                       goto fail;

SP.

>                                 __axl_log (LOG_DOMAIN,
> AXL_LEVEL_DEBUG, "finished parsing opened node, current parent=<%s>",
>                                            axl_node_get_name (node));
> @@ -1211,8 +1210,7 @@
>                         /* check for a null content found */
>                         if (string == NULL) {
>                                 axl_error_new (-1, "an error was found
> while reading the xml node content", stream, error);
> -                               axl_stream_free (stream);
> -                               return NULL;
> +                               goto fail;
>                         }

SP.
 
>                         /* nullify internal stream reference to have
> @@ -1235,12 +1233,7 @@
>                            axl_stack_size (doc->parentNode));
>  
>                 axl_error_new (-1, "XML document is not balanced,
> still remains xml nodes", stream, error);
> -
> -               /* parse complete */
> -               axl_stream_unlink (stream);
> -               axl_stream_free (stream);
> -
> -               return NULL;
> +               goto fail;

SP.

>         }
>  
>         /* parse complete */
> @@ -1253,6 +1246,11 @@
>         __axl_log (LOG_DOMAIN, AXL_LEVEL_DEBUG, "xml document parse
> COMPLETED"); 
>  
>         return doc;
> +fail:
> +       axl_stream_unlink (stream);
> +       axl_stream_free (stream);
> +       axl_doc_free (doc);
> +       return NULL;
>  }

Near to this change it is found a final check for unbalanced node stack
on close. This is a memory leak that could be activated if the document
have all nodes called the same and it is unbalanced at the end of the
document. This is now fixed and check by the RT.

Let me know if changes introduced fix memory leaks you are
experimenting. Thanks for reporting Benoit. 

Cheers!
-- 
Francis Brosnan Blazquez <francis at aspl.es>
Advanced Software Production Line, S.L.