[PATCH v9 5/5] eficonfig: add "Show Signature Database" menu entry

[Masahisa Kojima]

On Fri, 18 Nov 2022 at 11:18, Masahisa Kojima
<masahisa.kojima at linaro.org> wrote:
>
> Hi Ilias, Heinrich,
>
> On Fri, 18 Nov 2022 at 07:06, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >
> > On 11/17/22 22:49, Ilias Apalodimas wrote:
> > > Hi Kojima-san
> > >
> > >> +/**
> > >> + * prepare_signature_list_menu() - create the signature list menu entry
> > >> + *
> > >> + * @efimenu:        pointer to the efimenu structure
> > >> + * @varname:        pointer to the variable name
> > >> + * @db:             pointer to the variable raw data
> > >> + * @db_size:        variable data size
> > >> + * @func:   callback of each entry
> > >> + * Return:  status code
> > >> + */
> > >> +static efi_status_t prepare_signature_list_menu(struct efimenu *efi_menu, void *varname,
> > >> +                                            void *db, efi_uintn_t db_size,
> > >> +                                            eficonfig_entry_func func)
> > >> +{
> > >> +    u32 num = 0;
> > >> +    efi_uintn_t size;
> > >> +    struct eficonfig_sig_data *sg;
> > >> +    struct efi_signature_list *esl;
> > >> +    struct efi_signature_data *esd;
> > >> +    efi_status_t ret = EFI_SUCCESS;
> > >> +
> > >> +    INIT_LIST_HEAD(&efi_menu->list);
> > >> +
> > >> +    esl = db;
> > >> +    size = db_size;
> > >> +    while (size > 0) {
> > >> +            u32 remain;
> > >> +
> > >> +            esd = (struct efi_signature_data *)((u8 *)esl +
> > >> +                                                (sizeof(struct efi_signature_list) +
> > >> +                                                esl->signature_header_size));
> > >> +            remain = esl->signature_list_size - sizeof(struct efi_signature_list) -
> > >> +                     esl->signature_header_size;
> > >> +            for (; remain > 0; remain -= esl->signature_size) {
> > >> +                    char buf[40];
> > >
> > > This is going to hold a GUID right? Isn't buf[37] enough?
> > >
> > >> +                    char *title;
> > >> +
> > >> +                    if (num >= EFICONFIG_ENTRY_NUM_MAX - 1) {
> > >> +                            ret = EFI_OUT_OF_RESOURCES;
> > >> +                            goto out;
> > >> +                    }
> > >> +
> > >> +                    sg = calloc(1, sizeof(struct eficonfig_sig_data));
> > >> +                    if (!sg) {
> > >> +                            ret = EFI_OUT_OF_RESOURCES;
> > >> +                            goto err;
> > >> +                    }
> > >> +
> > >> +                    snprintf(buf, sizeof(buf), "%pUL", &esd->signature_owner);
> > >> +                    title = calloc(1, (strlen(buf) + 1));
> > >
> > > Since the length will always be static, just use sizeof(buf)
> >
> > You can use strdup() instead of strlen + calloc + strlcpy.
>
> buf[37] is enough and it is static, laos can use strdup() here.

Sorry but 'laos' is typo, please ignore this.

Regards,
Masahisa Kojima

>
> Thanks,
> Masahisa Kojima
>
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > >> +                    if (!title) {
> > >> +                            free(sg);
> > >> +                            ret = EFI_OUT_OF_RESOURCES;
> > >> +                            goto err;
> > >> +                    }
> > >> +                    strlcpy(title, buf, strlen(buf) + 1);
> > >> +
> > >> +                    sg->esl = esl;
> > >> +                    sg->esd = esd;
> > >> +                    sg->varname = varname;
> > >> +                    ret = eficonfig_append_menu_entry(efi_menu, title, func, sg);
> > >> +                    if (ret != EFI_SUCCESS) {
> > >> +                            free(sg);
> > >> +                            free(title);
> > >> +                            goto err;
> > >> +                    }
> > >> +                    esd = (struct efi_signature_data *)((u8 *)esd + esl->signature_size);
> > >> +                    num++;
> > >> +            }
> > >> +
> > >> +            size -= esl->signature_list_size;
> > >> +            esl = (struct efi_signature_list *)((u8 *)esl + esl->signature_list_size);
> > >> +    }
> > >> +out:
> > >> +    ret = eficonfig_append_quit_entry(efi_menu);
> > >> +err:
> > >> +    return ret;
> > >
> > > With those
> > > Acked-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > >
> > >
> >