On Tue, 28 May 2024 at 19:59, Heinrich Schuchardt
<heinrich.schucha...@canonical.com> wrote:
>
> On 28.05.24 18:18, Ilias Apalodimas wrote:
> > [...]
> >
> >>>> - unsigned sz2 = efi_dp_size(dp2);
> >>>> + size_t sz1;
> >>>> + size_t sz2 = efi_dp_size(dp2);
> >>>> void *p;
> >>>>
> >>>> + if (split_end_node < sizeof(struct efi_device_path))
> >>>
> >>> Can we be more explicit here pls? Someone might misuse this in the future
> >>> split_end_node < =1 ? And we can document we can use values up to
> >>> sizeof(struct efi_device_path) if we ever need extra functionality
> >>
> >> size_t split_end_node cannot be negative.
> >>
> >> The case split_end_node == 0 is handled below. What are you missing?
> >
> > someone misusing it and passing a value of '3' for example and print
> > an error if the value is
> > 1 < value < sizeof(struct efi_device_path)
>
> I would like to avoid over-engineering. How about
>
> - if (split_end_node < sizeof(struct efi_device_path))
> + if (split_end_node < 2)
>
> and changing the function description to refer to >= 2?
Nop, in that case, I prefer what's already there, simply because
values between 0 < sizeof(struct efi_device_path) will be used for
functionality similar to the one we have for 0,1 in the future
Thanks
/Ilias
>
> Best regards
>
> Heinrich
>
> >
> > Thanks
> > /Ilias
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >>>
> >>>> + sz1 = efi_dp_size(dp1);
> >>>> + else
> >>>> + sz1 = split_end_node;
> >>>> +
> >>>> if (split_end_node)
> >>>> end_size = 2 * sizeof(END);
> >>>> else
> >>>> diff --git a/lib/efi_loader/efi_device_path_utilities.c
> >>>> b/lib/efi_loader/efi_device_path_utilities.c
> >>>> index c95dbfa9b5f..ac250bbfcc9 100644
> >>>> --- a/lib/efi_loader/efi_device_path_utilities.c
> >>>> +++ b/lib/efi_loader/efi_device_path_utilities.c
> >>>> @@ -76,7 +76,7 @@ static struct efi_device_path * EFIAPI
> >>>> append_device_path(
> >>>> const struct efi_device_path *src2)
> >>>> {
> >>>> EFI_ENTRY("%pD, %pD", src1, src2);
> >>>> - return EFI_EXIT(efi_dp_concat(src1, src2, false));
> >>>> + return EFI_EXIT(efi_dp_concat(src1, src2, 0));
> >>>> }
> >>>>
> >>>> /*
> >>>> --
> >>>> 2.43.0
> >>>>
> >>>
> >>> Reviewed-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> >>
>
