[PATCH 1/2] discover/syslinux-parser: clean up boot option list entries

[Samuel Mendoza-Jonas]

On Thu, 2018-04-12 at 16:12 +1000, Brett Grandbois wrote:
> in finalize loop or we can get duplicate boot entries as well as
> the memory leak
> 
> Signed-off-by: Brett Grandbois <brett.grandbois at opengear.com>
> ---
>  discover/syslinux-parser.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/discover/syslinux-parser.c b/discover/syslinux-parser.c
> index d948765..8f5bfeb 100644
> --- a/discover/syslinux-parser.c
> +++ b/discover/syslinux-parser.c
> @@ -310,20 +310,21 @@ static void syslinux_finalize(struct conf_context *conf)
>  		implicit_image = false;
>  
>  	list_for_each_entry(&state->processed_options, syslinux_opt, list) {
> +		list_remove(&syslinux_opt->list);

Should be list_for_each_entry_safe() now.

>  		/* need a valid image */
>  		if (!syslinux_opt->image)
> -			continue;
> +			goto next;
>  
>  		image = syslinux_opt->image;
>  		label = syslinux_opt->label;
>  
>  		/* if implicit is disabled we must have a label */
>  		if (!label && !implicit_image)
> -			continue;
> +			goto next;
>  
>  		d_opt = discover_boot_option_create(dc, dc->device);
>  		if (!d_opt)
> -			continue;
> +			goto next;
>  		if (!d_opt->option)
>  			goto fail;
>  
> @@ -403,8 +404,11 @@ static void syslinux_finalize(struct conf_context *conf)
>  		conf_strip_str(opt->description);
>  
>  		discover_context_add_boot_option(dc, d_opt);
> +next:
> +		talloc_free(syslinux_opt);
>  		continue;
>  fail:
> +		talloc_free(syslinux_opt);
>  		talloc_free(d_opt);
>  	}

I wonder if it might be simpler to after the loop do

	list_for_each_entry_safe(&state->processed_options, syslinux_opt, list)
		talloc_free(syslinux_opt);
	list_init(&state->processed_options);

Then we just need talloc_free(d_opt) at the end of the first loop and we're
sure the list is empty before returning to syslinux_parse(). What do you think?

>  }