| Message ID | 20221110113610.231544-1-ilias.apalodimas@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | efi_loader: simplify efi_load_from_path | expand |
Heinrich Please ignore this. As we discussed although this cleans up things considerably, it's not exactly what the spec describes. So let's keep the existing function. I've sent another patch cleaning up the EFI_CALL() iterations Thanks /Ilias On Thu, 10 Nov 2022 at 13:36, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > The current implementation efi_load_from_path is a bit confusing. > First of all it tries to check the device path to make sure it contains > the path to the device plus the media path with the filename and nothing in > between. But that should already be valid since U-Boot constructs those > device paths. > On top of that it tries to traverse the device path nodes and acquire the > file part by stepping through the nodes of the directory path until the > file is reached. We already have efi_dp_split_file_path() for that so > rewrite the function and clean it up to use existing code. > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > lib/efi_loader/efi_file.c | 52 ++++++++++++--------------------------- > 1 file changed, 16 insertions(+), 36 deletions(-) > > diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c > index c96a7f7ca371..e1695ba309e4 100644 > --- a/lib/efi_loader/efi_file.c > +++ b/lib/efi_loader/efi_file.c > @@ -1104,53 +1104,33 @@ static const struct efi_file_handle efi_file_handle_protocol = { > struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) > { > struct efi_simple_file_system_protocol *v; > - struct efi_file_handle *f; > + struct efi_device_path_file_path *mdp; > + struct efi_device_path *dev_dp, *file_dp; > + struct efi_file_handle *vol_handle, *file_handle = NULL; > efi_status_t ret; > > v = efi_fs_from_path(fp); > if (!v) > return NULL; > > - EFI_CALL(ret = v->open_volume(v, &f)); > + ret = efi_dp_split_file_path(fp, &dev_dp, &file_dp); > if (ret != EFI_SUCCESS) > return NULL; > + efi_free_pool(dev_dp); > > - /* Skip over device-path nodes before the file path. */ > - while (fp && !EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) > - fp = efi_dp_next(fp); > - > - /* > - * Step through the nodes of the directory path until the actual file > - * node is reached which is the final node in the device path. > - */ > - while (fp) { > - struct efi_device_path_file_path *fdp = > - container_of(fp, struct efi_device_path_file_path, dp); > - struct efi_file_handle *f2; > - u16 *filename; > - > - if (!EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) { > - printf("bad file path!\n"); > - f->close(f); > - return NULL; > - } > - > - filename = u16_strdup(fdp->str); > - if (!filename) > - return NULL; > - EFI_CALL(ret = f->open(f, &f2, filename, > - EFI_FILE_MODE_READ, 0)); > - free(filename); > - if (ret != EFI_SUCCESS) > - return NULL; > - > - fp = efi_dp_next(fp); > - > - EFI_CALL(f->close(f)); > - f = f2; > + ret = efi_open_volume_int(v, &vol_handle); > + if (ret != EFI_SUCCESS) { > + efi_free_pool(file_dp); > + return NULL; > } > > - return f; > + mdp = (struct efi_device_path_file_path *)file_dp; > + /* we don't really care about the ret here *file_handle will be NULL */ > + ret = efi_file_open_int(vol_handle, &file_handle, mdp->str, EFI_FILE_MODE_READ, 0); > + efi_free_pool(file_dp); > + efi_file_close_int(vol_handle); > + > + return file_handle; > } > > efi_status_t efi_open_volume_int(struct efi_simple_file_system_protocol *this, > -- > 2.38.1 >
diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c index c96a7f7ca371..e1695ba309e4 100644 --- a/lib/efi_loader/efi_file.c +++ b/lib/efi_loader/efi_file.c @@ -1104,53 +1104,33 @@ static const struct efi_file_handle efi_file_handle_protocol = { struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) { struct efi_simple_file_system_protocol *v; - struct efi_file_handle *f; + struct efi_device_path_file_path *mdp; + struct efi_device_path *dev_dp, *file_dp; + struct efi_file_handle *vol_handle, *file_handle = NULL; efi_status_t ret; v = efi_fs_from_path(fp); if (!v) return NULL; - EFI_CALL(ret = v->open_volume(v, &f)); + ret = efi_dp_split_file_path(fp, &dev_dp, &file_dp); if (ret != EFI_SUCCESS) return NULL; + efi_free_pool(dev_dp); - /* Skip over device-path nodes before the file path. */ - while (fp && !EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) - fp = efi_dp_next(fp); - - /* - * Step through the nodes of the directory path until the actual file - * node is reached which is the final node in the device path. - */ - while (fp) { - struct efi_device_path_file_path *fdp = - container_of(fp, struct efi_device_path_file_path, dp); - struct efi_file_handle *f2; - u16 *filename; - - if (!EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) { - printf("bad file path!\n"); - f->close(f); - return NULL; - } - - filename = u16_strdup(fdp->str); - if (!filename) - return NULL; - EFI_CALL(ret = f->open(f, &f2, filename, - EFI_FILE_MODE_READ, 0)); - free(filename); - if (ret != EFI_SUCCESS) - return NULL; - - fp = efi_dp_next(fp); - - EFI_CALL(f->close(f)); - f = f2; + ret = efi_open_volume_int(v, &vol_handle); + if (ret != EFI_SUCCESS) { + efi_free_pool(file_dp); + return NULL; } - return f; + mdp = (struct efi_device_path_file_path *)file_dp; + /* we don't really care about the ret here *file_handle will be NULL */ + ret = efi_file_open_int(vol_handle, &file_handle, mdp->str, EFI_FILE_MODE_READ, 0); + efi_free_pool(file_dp); + efi_file_close_int(vol_handle); + + return file_handle; } efi_status_t efi_open_volume_int(struct efi_simple_file_system_protocol *this,
The current implementation efi_load_from_path is a bit confusing. First of all it tries to check the device path to make sure it contains the path to the device plus the media path with the filename and nothing in between. But that should already be valid since U-Boot constructs those device paths. On top of that it tries to traverse the device path nodes and acquire the file part by stepping through the nodes of the directory path until the file is reached. We already have efi_dp_split_file_path() for that so rewrite the function and clean it up to use existing code. Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- lib/efi_loader/efi_file.c | 52 ++++++++++++--------------------------- 1 file changed, 16 insertions(+), 36 deletions(-)