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

[Samuel Mendoza-Jonas]

On Wed, 2018-04-18 at 15:42 +1000, Brett Grandbois wrote:
> 
> On 18/04/18 15:01, Samuel Mendoza-Jonas wrote:
> > On Wed, 2018-04-18 at 10:20 +1000, Brett Grandbois wrote:
> > > in finalize loop or we can get duplicate boot entries as well as the
> > > memory leak
> > > ---
> > >   discover/syslinux-parser.c | 9 +++++++--
> > >   1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > Looks good but your Signed-off-by fell off in v2.
> 
> Do you want me to re-post with the signed off?  If so I can put back the 
> non-_safe list loops where they're not needed since I understand now 
> what the difference is.

Yes please :)

> > > diff --git a/discover/syslinux-parser.c b/discover/syslinux-parser.c
> > > index d948765..50e798a 100644
> > > --- a/discover/syslinux-parser.c
> > > +++ b/discover/syslinux-parser.c
> > > @@ -285,7 +285,7 @@ static void syslinux_process_pair(struct conf_context *conf, const char *name, c
> > >   static void syslinux_finalize(struct conf_context *conf)
> > >   {
> > >   	struct syslinux_options *state = conf->parser_info;
> > > -	struct syslinux_boot_option *syslinux_opt;
> > > +	struct syslinux_boot_option *syslinux_opt, *tmp;
> > >   	struct discover_context *dc = conf->dc;
> > >   	struct discover_boot_option *d_opt;
> > >   	bool implicit_image = true;
> > > @@ -309,7 +309,7 @@ static void syslinux_finalize(struct conf_context *conf)
> > >   	if (conf_get_global_option(conf, "implicit"), "0")
> > >   		implicit_image = false;
> > >   
> > > -	list_for_each_entry(&state->processed_options, syslinux_opt, list) {
> > > +	list_for_each_entry_safe(&state->processed_options, syslinux_opt, tmp, list) {
> > >   		/* need a valid image */
> > >   		if (!syslinux_opt->image)
> > >   			continue;
> > > @@ -404,9 +404,14 @@ static void syslinux_finalize(struct conf_context *conf)
> > >   
> > >   		discover_context_add_boot_option(dc, d_opt);
> > >   		continue;
> > > +
> > >   fail:
> > >   		talloc_free(d_opt);
> > >   	}
> > > +
> > > +	list_for_each_entry_safe(&state->processed_options, syslinux_opt, tmp, list)
> > > +		talloc_free(syslinux_opt);
> > > +	list_init(&state->processed_options);
> > >   }
> > >   
> > >   static int syslinux_parse(struct discover_context *dc)
> 
>