On 5/15/23 09:37, Ilias Apalodimas wrote:
Hi Heinrich,
Looking at this function can we clean it up since you are touching it already?
I think it would look nicer if you defined a local variable of struct
efi_device_path * and always assign it in the if cases.
Please, have a look at another patch in the series:
"efi_loader: simplify efi_dp_from_name()"
Then at the bottom of the function, we could store the ptr value if
that exists. While at it the 'if (!*file)' seems to be misplaced.
"if (!*file)" is not touched in this patch.
We cannot check the return value of efi_dp_from_file() before calling
the function. We have checked that that parameter file is non-null
above. Could you, please, describe what feels wrong for you.
Best regards
Heinrich
Regards
/Ilias
On Sat, 13 May 2023 at 11:48, Heinrich Schuchardt
<heinrich.schucha...@canonical.com> wrote:
According to our coding style guide #ifdef should be avoided.
Use IS_ENABLED() instead.
Sort string comparisons alphabetically.
Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
---
lib/efi_loader/efi_device_path.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index 20ad948498..a6a6ef0d6c 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -1079,8 +1079,7 @@ struct efi_device_path *efi_dp_from_uart(void)
return buf;
}
-#ifdef CONFIG_NETDEVICES
-struct efi_device_path *efi_dp_from_eth(void)
+struct efi_device_path __maybe_unused *efi_dp_from_eth(void)
{
void *buf, *start;
unsigned dpsize = 0;
@@ -1099,7 +1098,6 @@ struct efi_device_path *efi_dp_from_eth(void)
return start;
}
-#endif
/* Construct a device-path for memory-mapped image */
struct efi_device_path *efi_dp_from_mem(uint32_t memory_type,
@@ -1195,15 +1193,7 @@ efi_status_t efi_dp_from_name(const char *dev, const
char *devnr,
if (path && !file)
return EFI_INVALID_PARAMETER;
- if (!strcmp(dev, "Net")) {
-#ifdef CONFIG_NETDEVICES
- if (device)
- *device = efi_dp_from_eth();
-#endif
- } else if (!strcmp(dev, "Uart")) {
- if (device)
- *device = efi_dp_from_uart();
- } else if (!strcmp(dev, "Mem") || !strcmp(dev, "hostfs")) {
+ if (!strcmp(dev, "Mem") || !strcmp(dev, "hostfs")) {
/* loadm command and semihosting */
efi_get_image_parameters(&image_addr, &image_size);
@@ -1211,6 +1201,12 @@ efi_status_t efi_dp_from_name(const char *dev, const
char *devnr,
*device = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
(uintptr_t)image_addr,
image_size);
+ } else if (IS_ENABLED(CONFIG_NETDEVICES) && !strcmp(dev, "Net")) {
+ if (device)
+ *device = efi_dp_from_eth();
+ } else if (!strcmp(dev, "Uart")) {
+ if (device)
+ *device = efi_dp_from_uart();
} else {
part = blk_get_device_part_str(dev, devnr, &desc,
&fs_partition,
1);
--
2.39.2
