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@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox