Re: [edk2-devel] [PATCH] ShellPkg: Avoid corrupting installed device path protocols

["Ard Biesheuvel" <ardb@kernel.org>]

On Thu, 8 Dec 2022 at 22:15, Michael D Kinney
<michael.d.kinney@intel.com> wrote:
>
> Hi Ard,
>
> There is a difference between returning a pointer to a device path
> and modifying the device path contents.
>
> If you add CONST to the argument, then an updated pointer to a device
> path can not be returned.
>

No, this is incorrect.

The function takes a pointer (1) to a pointer(2)  to a device path protocol

EFI_DEVICE_PATH_PROTOCOL **

So the function can dereference pointer 1 and modify pointer 2
*unless* it is marked as CONST, i.e.

EFI_DEVICE_PATH_PROTOCOL * CONST *

in which case the pointer is not modifiable, but it is permitted to
dereference that pointer to modify the underlying object.

I am arguing that the prototype should be

EFI_DEVICE_PATH_PROTOCOL CONST **

(which is the same as putting the CONST at the beginning)

where the caller's pointer can be advanced by the callee via the
pointer-to-pointer. But that would still not permit the object to be
modified.

> The API clear describes returning an updated device path pointer, so
> the API is declared correctly without CONST.
>

The pointer may be updated but not the object. It really comes down to
the difference between

CONST EFI_DEVICE_PATH_PROTOCOL **
EFI_DEVICE_PATH_PROTOCOL CONST **
EFI_DEVICE_PATH_PROTOCOL * CONST *
EFI_DEVICE_PATH_PROTOCOL **CONST

(where the first two mean the same thing0

> The API does not state that the contents of the device path are modified.
>
> An API that uses CONST EFI_DEVICE_PATH* would indicate that the API
> should not modify the contents of the device path.  For example:
>
> /**
>   Returns the size of a device path in bytes.
>
>   This function returns the size, in bytes, of the device path data structure
>   specified by DevicePath including the end of device path node.
>   If DevicePath is NULL or invalid, then 0 is returned.
>
>   @param  DevicePath  A pointer to a device path data structure.
>
>   @retval 0           If DevicePath is NULL or invalid.
>   @retval Others      The size of a device path in bytes.
>
> **/
> UINTN
> EFIAPI
> GetDevicePathSize (
>   IN CONST EFI_DEVICE_PATH_PROTOCOL  *DevicePath
>   );
>

Yes, but this one is a pointer, not a pointer-to-pointer. Big difference.