On Wed, 24 Aug 2022 at 20:15, Ilias Apalodimas
<ilias.apalodi...@linaro.org> wrote:
>
> Hi Kojima-san,
>
> [...]
>
> > + * eficonfig_destroy() - destroy efimenu
> > + *
> > + * @efi_menu: pointer to the efimenu structure
> > + * @flag: flag to free the allocated data
> > + */
> > +static void eficonfig_destroy(struct efimenu *efi_menu, bool flag)
> > +{
> > + struct list_head *pos, *n;
> > + struct eficonfig_entry *entry;
> > +
> > + list_for_each_safe(pos, n, &efi_menu->list) {
> > + entry = list_entry(pos, struct eficonfig_entry, list);
> > + free(entry->title);
> > + if (flag)
> > + free(entry->data);
>
> I don't we need this flag. entry->data is either set to a valid pointer or
> NULL on append_entry().
There is a case that entry->data is still used after the menu is destroyed.
The following case, file_info is still used after eficonfig_destroy().
https://git.linaro.org/people/masahisa.kojima/u-boot.git/tree/cmd/eficonfig.c?h=kojima/eficonfig_upstream_v13#n953
Also there are the cases that a not-allocated pointer is set to entry->data.
In addition, this flag will be used to handle the following static menu entry
for secure boot key management series.
+ {"PK", eficonfig_process_set_secure_boot_pk, u"PK"},
+ {"KEK", eficonfig_process_set_secure_boot_key, u"KEK"},
+ {"db", eficonfig_process_set_secure_boot_key, u"db"},
+ {"dbx", eficonfig_process_set_secure_boot_key, u"dbx"},
+ {"Quit", eficonfig_process_quit},
The last parameter(u"PK", u"KEK") is set to entry->data, it will not be freed.
But this flag is a little tricky.
Thanks,
Masahisa Kojima
>
> > + list_del(&entry->list);
> > + free(entry);
> > + }
> > + free(efi_menu->menu_header);
> > + free(efi_menu);
>
> [...]
>
> Regards
> /Ilias
