[PATCH] Terminate decode at end of string and fix mem leak

[Dirk Hohndel]

Miika Turkia <miika.turkia at gmail.com> writes:

> Seems that we have to NULL terminate the buffer for
> xmlStringLenDecodeEntitites() as otherwise we might end up having extra
> data at the end of returned buffer.  (Somehow the length parameter is
> not respected always, even if it is the proper size returned by the
> zip_fread() - header_skip).
>
> Also free the buffer returned by xmlStringLenDecodeEntitites().
>
> Signed-off-by: Miika Turkia <miika.turkia at gmail.com>
> ---
>  file.c      |    1 +
>  parse-xml.c |   13 +++++++++----
>  2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/file.c b/file.c
> index b5fba30..8deb2fe 100644
> --- a/file.c
> +++ b/file.c
> @@ -72,6 +72,7 @@ static void zip_read(struct zip_file *file, GError **error, const char *filename
>  		size = read * 3 / 2;
>  		mem = realloc(mem, size);
>  	}
> +	mem[read] = 0;
>  	parse_xml_buffer(filename, mem, read, &dive_table, error);
>  	free(mem);
>  }
> diff --git a/parse-xml.c b/parse-xml.c
> index b24806b..6dd3094 100644
> --- a/parse-xml.c
> +++ b/parse-xml.c
> @@ -1538,7 +1538,7 @@ static void reset_all(void)
>   * but once we decode the HTML encoded characters they turn
>   * into UTF-8 instead. So skip the incorrect encoding
>   * declaration and decode the HTML encoded characters */
> -const char *preprocess_divelog_de(const char *buffer)
> +char *preprocess_divelog_de(const char *buffer)

why remove the const?

>  {
>  	char *ret = strstr(buffer, "<DIVELOGSDATA>");
>  
> @@ -1551,17 +1551,22 @@ const char *preprocess_divelog_de(const char *buffer)
>  
>  		return ret;
>  	}
> -	return buffer;
> +	return NULL;

Why return NULL in this case...

>  }
>  
>  void parse_xml_buffer(const char *url, const char *buffer, int size,
>  			struct dive_table *table, GError **error)
>  {
>  	xmlDoc *doc;
> -	const char *res = preprocess_divelog_de(buffer);
> +	char *res = preprocess_divelog_de(buffer);
>  
>  	target_table = table;
> -	doc = xmlReadMemory(res, strlen(res), url, NULL, 0);
> +	if (res) {
> +		doc = xmlReadMemory(res, strlen(res), url, NULL, 0);
> +		free(res);
> +	} else
> +		doc = xmlReadMemory(buffer, size, url, NULL, 0);
> +

and then have the if statement here?

I'm missing the point why this is better (and I dislike the asymetric "}
else" -- if you one side has curly braces, the other one should, too...

/D