| Message ID | 20221120002119.23683-5-masahisa.kojima@linaro.org |
|---|---|
| State | Accepted |
| Commit | d6566113102e852937d0845a528337484ae85f95 |
| Headers | show |
| Series | eficonfig: add UEFI Secure Boot key maintenance interface | expand |
On 11/20/22 01:21, Masahisa Kojima wrote: > Following commits are adding support for UEFI variable management > via the eficonfig menu. Those functions needs to use > eficonfig_create_device_path() to construct the full device path > from device path of the volume and file path, so move it > out of their static declarations. > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > --- > Newly created in v10 > > cmd/eficonfig.c | 20 +++++++++++--------- > include/efi_config.h | 2 ++ > 2 files changed, 13 insertions(+), 9 deletions(-) > > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c > index 12babb76c2..58a6856666 100644 > --- a/cmd/eficonfig.c > +++ b/cmd/eficonfig.c > @@ -436,14 +436,15 @@ static efi_status_t eficonfig_volume_selected(void *data) > } > > /** > - * create_selected_device_path() - create device path > + * eficonfig_create_device_path() - create device path > * > - * @file_info: pointer to the selected file information > + * @dp_volume: pointer to the volume > + * @current_path: pointer to the file path u16 string > * Return: > * device path or NULL. Caller must free the returned value > */ > -static > -struct efi_device_path *create_selected_device_path(struct eficonfig_select_file_info *file_info) > +struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume, > + u16 *current_path) > { > char *p; > void *buf; > @@ -452,7 +453,7 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file > struct efi_device_path_file_path *fp; This function should be moved to lib/efi_loader/efi_device_path.c so that we can use it to simplify efi_dp_from_file(). > > fp_size = sizeof(struct efi_device_path) + > - ((u16_strlen(file_info->current_path) + 1) * sizeof(u16)); > + ((u16_strlen(current_path) + 1) * sizeof(u16)); You could reduce the code size a little with: fp_size = sizeof(struct efi_device_path) + u16_strsize(file_info->current_path); Best regards Heinrich > buf = calloc(1, fp_size + sizeof(END)); > if (!buf) > return NULL; > @@ -461,13 +462,13 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file > fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE, > fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH, > fp->dp.length = (u16)fp_size; > - u16_strcpy(fp->str, file_info->current_path); > + u16_strcpy(fp->str, current_path); > > p = buf; > p += fp_size; > *((struct efi_device_path *)p) = END; > > - dp = efi_dp_append(file_info->dp_volume, (struct efi_device_path *)buf); > + dp = efi_dp_append(dp_volume, (struct efi_device_path *)buf); > free(buf); > > return dp; > @@ -1472,7 +1473,8 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo > } > > if (bo->initrd_info.dp_volume) { > - dp = create_selected_device_path(&bo->initrd_info); > + dp = eficonfig_create_device_path(bo->initrd_info.dp_volume, > + bo->initrd_info.current_path); > if (!dp) { > ret = EFI_OUT_OF_RESOURCES; > goto out; > @@ -1481,7 +1483,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo > efi_free_pool(dp); > } > > - dp = create_selected_device_path(&bo->file_info); > + dp = eficonfig_create_device_path(bo->file_info.dp_volume, bo->file_info.current_path); > if (!dp) { > ret = EFI_OUT_OF_RESOURCES; > goto out; > diff --git a/include/efi_config.h b/include/efi_config.h > index 064f2efe3f..934de41e85 100644 > --- a/include/efi_config.h > +++ b/include/efi_config.h > @@ -99,5 +99,7 @@ efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu, > char *title, eficonfig_entry_func func, > void *data); > efi_status_t eficonfig_append_quit_entry(struct efimenu *efi_menu); > +struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume, > + u16 *current_path); > > #endif
Hi Heinrich, On Sun, 20 Nov 2022 at 20:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 11/20/22 01:21, Masahisa Kojima wrote: > > Following commits are adding support for UEFI variable management > > via the eficonfig menu. Those functions needs to use > > eficonfig_create_device_path() to construct the full device path > > from device path of the volume and file path, so move it > > out of their static declarations. > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > > --- > > Newly created in v10 > > > > cmd/eficonfig.c | 20 +++++++++++--------- > > include/efi_config.h | 2 ++ > > 2 files changed, 13 insertions(+), 9 deletions(-) > > > > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c > > index 12babb76c2..58a6856666 100644 > > --- a/cmd/eficonfig.c > > +++ b/cmd/eficonfig.c > > @@ -436,14 +436,15 @@ static efi_status_t eficonfig_volume_selected(void *data) > > } > > > > /** > > - * create_selected_device_path() - create device path > > + * eficonfig_create_device_path() - create device path > > * > > - * @file_info: pointer to the selected file information > > + * @dp_volume: pointer to the volume > > + * @current_path: pointer to the file path u16 string > > * Return: > > * device path or NULL. Caller must free the returned value > > */ > > -static > > -struct efi_device_path *create_selected_device_path(struct eficonfig_select_file_info *file_info) > > +struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume, > > + u16 *current_path) > > { > > char *p; > > void *buf; > > @@ -452,7 +453,7 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file > > struct efi_device_path_file_path *fp; > > This function should be moved to lib/efi_loader/efi_device_path.c so > that we can use it to simplify efi_dp_from_file(). eficonfig_create_device_path() takes u16 file path string, but efi_dp_from_file() takes file path string, so additional charset conversion is required to call this library function from efi_dp_from_file() and I wonder it simplifies the code. > > > > > fp_size = sizeof(struct efi_device_path) + > > - ((u16_strlen(file_info->current_path) + 1) * sizeof(u16)); > > + ((u16_strlen(current_path) + 1) * sizeof(u16)); > > You could reduce the code size a little with: > > fp_size = sizeof(struct efi_device_path) + > u16_strsize(file_info->current_path); I'm aware that you have already merged this series. I will send a follow up patch to fix this u16_strsize(). Thanks, Masahisa Kojima > > Best regards > > Heinrich > > > buf = calloc(1, fp_size + sizeof(END)); > > if (!buf) > > return NULL; > > @@ -461,13 +462,13 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file > > fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE, > > fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH, > > fp->dp.length = (u16)fp_size; > > - u16_strcpy(fp->str, file_info->current_path); > > + u16_strcpy(fp->str, current_path); > > > > p = buf; > > p += fp_size; > > *((struct efi_device_path *)p) = END; > > > > - dp = efi_dp_append(file_info->dp_volume, (struct efi_device_path *)buf); > > + dp = efi_dp_append(dp_volume, (struct efi_device_path *)buf); > > free(buf); > > > > return dp; > > @@ -1472,7 +1473,8 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo > > } > > > > if (bo->initrd_info.dp_volume) { > > - dp = create_selected_device_path(&bo->initrd_info); > > + dp = eficonfig_create_device_path(bo->initrd_info.dp_volume, > > + bo->initrd_info.current_path); > > if (!dp) { > > ret = EFI_OUT_OF_RESOURCES; > > goto out; > > @@ -1481,7 +1483,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo > > efi_free_pool(dp); > > } > > > > - dp = create_selected_device_path(&bo->file_info); > > + dp = eficonfig_create_device_path(bo->file_info.dp_volume, bo->file_info.current_path); > > if (!dp) { > > ret = EFI_OUT_OF_RESOURCES; > > goto out; > > diff --git a/include/efi_config.h b/include/efi_config.h > > index 064f2efe3f..934de41e85 100644 > > --- a/include/efi_config.h > > +++ b/include/efi_config.h > > @@ -99,5 +99,7 @@ efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu, > > char *title, eficonfig_entry_func func, > > void *data); > > efi_status_t eficonfig_append_quit_entry(struct efimenu *efi_menu); > > +struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume, > > + u16 *current_path); > > > > #endif >
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c index 12babb76c2..58a6856666 100644 --- a/cmd/eficonfig.c +++ b/cmd/eficonfig.c @@ -436,14 +436,15 @@ static efi_status_t eficonfig_volume_selected(void *data) } /** - * create_selected_device_path() - create device path + * eficonfig_create_device_path() - create device path * - * @file_info: pointer to the selected file information + * @dp_volume: pointer to the volume + * @current_path: pointer to the file path u16 string * Return: * device path or NULL. Caller must free the returned value */ -static -struct efi_device_path *create_selected_device_path(struct eficonfig_select_file_info *file_info) +struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume, + u16 *current_path) { char *p; void *buf; @@ -452,7 +453,7 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file struct efi_device_path_file_path *fp; fp_size = sizeof(struct efi_device_path) + - ((u16_strlen(file_info->current_path) + 1) * sizeof(u16)); + ((u16_strlen(current_path) + 1) * sizeof(u16)); buf = calloc(1, fp_size + sizeof(END)); if (!buf) return NULL; @@ -461,13 +462,13 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE, fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH, fp->dp.length = (u16)fp_size; - u16_strcpy(fp->str, file_info->current_path); + u16_strcpy(fp->str, current_path); p = buf; p += fp_size; *((struct efi_device_path *)p) = END; - dp = efi_dp_append(file_info->dp_volume, (struct efi_device_path *)buf); + dp = efi_dp_append(dp_volume, (struct efi_device_path *)buf); free(buf); return dp; @@ -1472,7 +1473,8 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo } if (bo->initrd_info.dp_volume) { - dp = create_selected_device_path(&bo->initrd_info); + dp = eficonfig_create_device_path(bo->initrd_info.dp_volume, + bo->initrd_info.current_path); if (!dp) { ret = EFI_OUT_OF_RESOURCES; goto out; @@ -1481,7 +1483,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo efi_free_pool(dp); } - dp = create_selected_device_path(&bo->file_info); + dp = eficonfig_create_device_path(bo->file_info.dp_volume, bo->file_info.current_path); if (!dp) { ret = EFI_OUT_OF_RESOURCES; goto out; diff --git a/include/efi_config.h b/include/efi_config.h index 064f2efe3f..934de41e85 100644 --- a/include/efi_config.h +++ b/include/efi_config.h @@ -99,5 +99,7 @@ efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu, char *title, eficonfig_entry_func func, void *data); efi_status_t eficonfig_append_quit_entry(struct efimenu *efi_menu); +struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume, + u16 *current_path); #endif
Following commits are adding support for UEFI variable management via the eficonfig menu. Those functions needs to use eficonfig_create_device_path() to construct the full device path from device path of the volume and file path, so move it out of their static declarations. Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> --- Newly created in v10 cmd/eficonfig.c | 20 +++++++++++--------- include/efi_config.h | 2 ++ 2 files changed, 13 insertions(+), 9 deletions(-)