Re: [PATCH 9/9] menu: simplify usage for clients

[Sascha Hauer <s.hauer@pengutronix.de>]

On Mon, Aug 23, 2010 at 05:18:05PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 08:24 Mon 23 Aug     , Sascha Hauer wrote:
> > Clients now only have to call menu_add_submenu or menu_add_command_entry
> > instead of allocating many strings.
> > This also fixes some problems in the menu code. The priv field in struct
> > menu_entry was a pointer to struct menu or a pointer to an allocated string.
> > It was never freed, only had to be freed when it was an allocated string.
> > The reference to a submenu is now kept as a string and not to the menu
> > itself. The code checked the existence of the submenu when creating it, but
> > crashed when the submenu was removed and referenced afterwards. Now the
> > code hapily allows references to nonexistant menus but complains during
> > runtime when the menu is not there.
> ok but no need to check if the pointer is null before freeing and please do
> not remove the priv pointer as I use is in C API for complex menu to pass data
> to the action
> That's why I keep it as void*

So there's data associated to *priv, how do you free it then when...

> > 
> >  #include <errno.h>
> >  #include <readkey.h>
> > +#include <linux/err.h>
> >  
> >  static LIST_HEAD(menus);
> >  
> > @@ -145,10 +146,7 @@ void menu_entry_free(struct menu_entry *me)
> >  	if (!me)
> >  		return;
> >  
> > -	if (me->display)
> > -		free(me->display);
> > -
> > -	free(me);
> > +	me->free(me);
> we must check the pointer first as in the C API we must be able to overwrite
> it or we must check that the C API provice it

... you don't provide a free function?

You can't overwrite menu_entry_free as it is called from menu_free.

> >  }
> >  
> >  static void print_menu_entry(struct menu *m, struct menu_entry *me, int reverse)
> > @@ -277,14 +275,76 @@ int menu_show(struct menu *m)
> >  
> >  void menu_action_exit(struct menu *m, struct menu_entry *me) {}
> >  
> > -void menu_action_run(struct menu *m, struct menu_entry *me)
> > +struct submenu {
> > +	char *submenu;
> > +	struct menu_entry entry;
> > +};

Note how struct menu_entry here is contained in a bigger struct. This
way you can associate private data to a menu_entry using container_of
without the use of a priv pointer.

That said we can keep the priv pointer, but you can't use it for data
which must be freed, at least not without providing a free function.
That was what I was trying to enforce, maybe that went a bit too far.

Ok for the NULL pointer checks before free. I will remove them.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox