On 04/16/20 09:33, Guomin Jiang wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2652 > > According to the description, the File is OPTIONAL and can be NULL. > > Signed-off-by: Guomin Jiang <guomin.ji...@intel.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Liming Gao <liming....@intel.com> > --- > MdePkg/Include/Protocol/Security2.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MdePkg/Include/Protocol/Security2.h > b/MdePkg/Include/Protocol/Security2.h > index 2d85b4ba9f..6a63009956 100644 > --- a/MdePkg/Include/Protocol/Security2.h > +++ b/MdePkg/Include/Protocol/Security2.h > @@ -80,7 +80,7 @@ typedef struct _EFI_SECURITY2_ARCH_PROTOCOL > EFI_SECURITY2_ARCH_PROTOCOL; > **/ > typedef EFI_STATUS (EFIAPI *EFI_SECURITY2_FILE_AUTHENTICATION) ( > IN CONST EFI_SECURITY2_ARCH_PROTOCOL *This, > - IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePath, > + IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePath, OPTIONAL > IN VOID *FileBuffer, > IN UINTN FileSize, > IN BOOLEAN BootPolicy >
(1) I think writing DevicePath OPTIONAL, is a bit more idiomatic than DevicePath, OPTIONAL However, the UEFI spec contains examples for both styles of comma placement in parameter lists; and so I think this change is valid, albeit somewhat unusual. So that's good. (2) Unfortunately, I don't think the patch is complete. If you look at the entire function declaration in the header file, the documentation *inconsistently* calls the parameter both "File" and "DevicePath". If you search the leading comment, there are both "File" and "DevicePath" references. Therefore the reference in the commit message, "According to the description, the File is OPTIONAL", is actually a dangling reference, because *that* particular part of the documentation calls the parameter "File". Please rework this patch so that the documentation and the parameter list fully agree on the parameter's name. According to the platform init spec (1.7), EFI_SECURITY2_ARCH_PROTOCOL.FileAuthentication(), the name of the parameter is "DevicePath". So please replace all relevant "File" references in the leading comment with "DevicePath". (3) Also, I think it would be prudent to file a ticket in Mantis, to mark "DevicePath" as OPTIONAL in the spec too. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57620): https://edk2.groups.io/g/devel/message/57620 Mute This Topic: https://groups.io/mt/73050539/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
