On Tue, 19 Nov 2024 at 11:50, Heinrich Schuchardt <[email protected]> wrote: > > On 18.11.24 22:08, Adriano Cordova wrote: > > Add efi_dp_from_ipv4 to form a device path from an ipv4 address. > > > > Signed-off-by: Adriano Cordova <[email protected]> > > --- > > > > Changes in v4: > > - Fixed memcpy mistake > > > > Changes in v3: > > - Removed some unnecessary void* casts. > > - Changed sizeof(struct efi_device_path_ipv4) to sizeof(*ipv4dp) > > in efi_dp_from_ipv4. > > > > lib/efi_loader/efi_device_path.c | 38 ++++++++++++++++++++++++++++++++ > > 1 file changed, 38 insertions(+) > > > > diff --git a/lib/efi_loader/efi_device_path.c > > b/lib/efi_loader/efi_device_path.c > > index ee387e1dfd..cdeea4791f 100644 > > --- a/lib/efi_loader/efi_device_path.c > > +++ b/lib/efi_loader/efi_device_path.c > > @@ -974,6 +974,44 @@ struct efi_device_path __maybe_unused > > *efi_dp_from_eth(void) > > return start; > > } > > > > A function description is missing. > > > +struct efi_device_path *efi_dp_from_ipv4(struct efi_ipv4_address *ip, > > + struct efi_ipv4_address *mask, > > + struct efi_ipv4_address *srv) > > +{ > > + struct efi_device_path *dp1, *dp2; > > + efi_uintn_t ipv4dp_len; > > + struct efi_device_path_ipv4 *ipv4dp; > > + char *pos; > > + > > + ipv4dp_len = sizeof(*ipv4dp); > > + ipv4dp = efi_alloc(ipv4dp_len + sizeof(END)); > > ipv4dp is a small structure and is freed below. > Allocating it on the stack would reduce the code size. > > > + if (!ipv4dp) { > > + log_err("Out of memory\n"); > > Writing messages inside a protocol implementation should be avoided. > > > + return NULL; > > + } > > + > > + ipv4dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; > > + ipv4dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_IPV4; > > + ipv4dp->dp.length = ipv4dp_len; > > + ipv4dp->protocol = 6; > > + if (ip) > > + memcpy(&ipv4dp->local_ip_address, ip, sizeof(*ip)); > > + if (mask) > > + memcpy(&ipv4dp->subnet_mask, mask, sizeof(*mask)); > > + if (srv) > > + memcpy(&ipv4dp->remote_ip_address, srv, sizeof(*srv)); > > + pos = (void *)(uintptr_t)ipv4dp + ipv4dp_len; > > Here you > > 1 - convert ipv4dp to uintptr_t > 2 - convert the uintptr_t value to void * > 3 - add an offset to the void * value > > In the C standard adding an offset to a void * value results in > undefined behavior. Unfortunately U-Boot code uses it a lot.
It's a known support gcc extension that treats void* similar to char*. Do you think it's worth changing all of the callsites we have? Thanks /Ilias > > pos = (char *)ipv4dp + ipv4dp_len; > > is all it takes to have a defined behavior here. > > Best regards > > Heinrich > > > + memcpy(pos, &END, sizeof(END)); > > + > > + dp1 = efi_dp_from_eth(); > > + dp2 = efi_dp_concat(dp1, (const struct efi_device_path *)ipv4dp, 0); > > + > > + efi_free_pool(ipv4dp); > > + efi_free_pool(dp1); > > + > > + return dp2; > > +} > > + > > /* Construct a device-path for memory-mapped image */ > > struct efi_device_path *efi_dp_from_mem(uint32_t memory_type, > > uint64_t start_address, >
