[PATCH v7 4/5] eficonfig: add UEFI Secure Boot Key enrollment interface

[Masahisa Kojima]

On Thu, 10 Nov 2022 at 15:47, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> [...]
>
> > > > +             goto out;
> > > > +
> > > > +     ret = efi_open_volume_int(file_info.current_volume, &root);
> > > > +     if (ret != EFI_SUCCESS)
> > > > +             goto out;
> > > > +
> > > > +     ret = efi_file_open_int(root, &f, file_info.current_path, EFI_FILE_MODE_READ, 0);
> > > > +     if (ret != EFI_SUCCESS)
> > > > +             goto out;
> > >
> > > I think it would be better here if we could use efi_file_from_path().
> > > I think we can't easily do that atm since we can't convert the filename to
> > > a device path with efi_dp_from_file() since we don't have the block info.
> >
> > Here we have a device path of volume(file_info.current_volume) and
> > filename(file_info.current_path), so we can create a full device path to call
> > efi_file_from_path().
> >    # cmd/eficonfig.c::create_selected_device_path() create the full device path,
> >       we can reuse it.
> >
> > >
> > > Since that requires a further clean up, I am fine keeping it as-is for now,
> > > but add a comment saying we should replace that with efi_file_from_path()
> > > eventually.
> >
> > Probably I don't understand what is improved when we replace current code
> > with efi_file_from_path().
>
> I just prefer using common functions to open a file, rather than open
> coding open_volume + file_open. OTOH efi_file_from_path() just
> converts that DP into a filepath and reads the file.  So on a second
> thought leave this as is, we don't need a comment.

Thank you for the confirmation.

Regards,
Masahisa Kojima

>
> [...]
>
> Regards
> /Ilias