Amit, If consumer calls OpenProtocol with Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL and Interface = NULL, and the protocol could not be found, how could the code with the patch work to assign *Interface = NULL?
And also UEFI spec has words Interface will be ignored if Attributes is EFI_OPEN_PROTOCOL_TEST_PROTOCOL. “InterfaceSupplies the address where a pointer to the corresponding Protocol Interface is returned. NULL will be returned in *Interface if a structure is not associated with Protocol. This parameter is optional, and will be ignored if Attributes is EFI_OPEN_PROTOCOL_TEST_PROTOCOL.” Thanks, Star From: Amit Kumar [mailto:[email protected]] Sent: Thursday, June 22, 2017 8:54 PM To: [email protected]; Gao, Liming <[email protected]>; Zeng, Star <[email protected]>; Kinney, Michael D <[email protected]> Cc: Tian, Feng <[email protected]> Subject: RE: RE: [edk2] [PATCH V3] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol >>Should the code >> >>+ // Return NULL Interface if Unsupported Protocol >>+ *Interface = NULL; >> >>be >> >>+ if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) { >>+ // Return NULL Interface if Unsupported Protocol >>+ *Interface = NULL; >>+ } >> >>to cover the case Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL and Interface >>= NULL? >> >> >>Thanks, >>Star Hello Star, It looks like and you suggestion is apt and satisfies the use case scenarios of EFI_OPEN_PROTOCOL_TEST_PROTOCOL. But if we talk of UEFI Spec 2.7, I think the patch is good enough . I would like to have some suggestion here from all the concerned people, if agreed I would add these changes to the patch. Thanks And Regards Amit --------- Original Message --------- Sender : Zeng, Star <[email protected]<mailto:[email protected]>> Date : 2017-06-20 15:28 (GMT+5:30) Title : RE: [edk2] [PATCH V3] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol To : Amit Kumar<[email protected]<mailto:[email protected]>>, null<[email protected]<mailto:[email protected]>> CC : null<[email protected]<mailto:[email protected]>>, null<[email protected]<mailto:[email protected]>> Should the code + // Return NULL Interface if Unsupported Protocol + *Interface = NULL; be + if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) { + // Return NULL Interface if Unsupported Protocol + *Interface = NULL; + } to cover the case Attributes = EFI_OPEN_PROTOCOL_TEST_PROTOCOL and Interface = NULL? Thanks, Star -----Original Message----- From: edk2-devel [mailto:[email protected]] On Behalf Of Amit Kumar Sent: Monday, June 19, 2017 8:24 PM To: [email protected]<mailto:[email protected]> Cc: Tian, Feng <[email protected]<mailto:[email protected]>> Subject: [edk2] [PATCH V3] MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol Change Since v2: 1) Modified to use EFI_ERROR to get status code Change since v1: 1) Fixed typo protocal to protocol 2) Fixed coding style Modified source code to update Interface as per spec. 1) In case of Protocol is un-supported, interface should be returned NULL. 2) In case of any error, interface should not be modified. 3) In case of Test Protocol, interface is optional. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Amit Kumar <[email protected]<mailto:[email protected]>> --- MdeModulePkg/Core/Dxe/Hand/Handle.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c b/MdeModulePkg/Core/Dxe/Hand/Handle.c index 1c25521..6de300f 100644 --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c @@ -1004,12 +1004,8 @@ CoreOpenProtocol ( // // Check for invalid Interface // - if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) { - if (Interface == NULL) { - return EFI_INVALID_PARAMETER; - } else { - *Interface = NULL; - } + if ((Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) && (Interface == NULL)) { + return EFI_INVALID_PARAMETER; } // @@ -1073,15 +1069,11 @@ CoreOpenProtocol ( Prot = CoreGetProtocolInterface (UserHandle, Protocol); if (Prot == NULL) { Status = EFI_UNSUPPORTED; + // Return NULL Interface if Unsupported Protocol + *Interface = NULL; goto Done; } - // - // This is the protocol interface entry for this protocol - // - if (Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) { - *Interface = Prot->Interface; - } Status = EFI_SUCCESS; ByDriver = FALSE; @@ -1175,6 +1167,14 @@ CoreOpenProtocol ( } Done: + + // + // This is the protocol interface entry for this protocol. + // In case of any Error, Interface should not be updated as per spec. + // + if (!EFI_ERROR (Status) && Attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) { + *Interface = Prot->Interface; + } // // Done. Release the database lock are return // -- 1.9.1 _______________________________________________ edk2-devel mailing list [email protected]<mailto:[email protected]> https://lists.01.org/mailman/listinfo/edk2-devel [cid:[email protected]] [http://ext.samsung.net/mail/ext/v1/external/status/update?userid=amit.ak&do=bWFpbElEPTIwMTcwNjIyMTI1NDE5ZXBjbXM1cDM4NDkyNjBiOGMyNmYyMzM1MjJlMTYwYzlhMzQ2OTMzZSZyZWNpcGllbnRBZGRyZXNzPXN0YXIuemVuZ0BpbnRlbC5jb20_] _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
