Good suggestion. I agree both. I will update when I check in.
-----Original Message----- From: Long, Qin [mailto:[email protected]] Sent: Tuesday, April 28, 2015 9:52 AM To: [email protected] Subject: Re: [edk2] [PATCH] UEFI2.5 HASH2 The patch looks good to me. Could you add some descriptions about EFI_HASH2_PROTOCOL in the Hash2.h? There are only some descriptions about EFI_HASH2_SERVICE_BINDING_PROTOCOL now. In the driver implementation: +#define HASH2_SERVICE_DATA_SIGNATURE SIGNATURE_32 ('H', 's', '2', 'S') I prefer to use the same-case letter for signature: ('H', 'S', '2', 'S') Reviewed-by: Long, Qin ([email protected]) Best Regards & Thanks, LONG, Qin -----Original Message----- From: Yao, Jiewen [mailto:[email protected]] Sent: Tuesday, April 28, 2015 9:31 AM To: '[email protected]' Subject: Re: [edk2] [PATCH] UEFI2.5 HASH2 Hi Many thanks to Long Qin, who found an error in EFI_HASH2_OUTPUT definition in Hash2.h. Here is new patch. Thank you Yao Jiewen -----Original Message----- From: Yao, Jiewen Sent: Monday, April 27, 2015 8:38 AM To: Justen, Jordan L; [email protected] Subject: RE: [edk2] [PATCH] UEFI2.5 HASH2 Thanks for reminder. Commit message: 1) MdePkg/Include/IndustryStandard: Add UEFI2.5 defined EFI_HASH2_PROTOCOL definition. 2) SecurityPkg/Hash2DxeCrypto: Add UEFI2.5 defined EFI_HASH2_PROTOCOL implementation. Yes, it is written from scratch. There is no open source HASH protocol before. Thank you Yao Jiewen -----Original Message----- From: Justen, Jordan L Sent: Monday, April 27, 2015 5:03 AM To: [email protected]; Yao, Jiewen Subject: Re: [edk2] [PATCH] UEFI2.5 HASH2 On 2015-04-25 20:49:55, Yao, Jiewen wrote: > Hello > > Here is patch to add UEFI2.5 HASH2 protocol. > > Patch 1: Add HASH2 protocol. > > Patch 2: Add HASH2 driver. Is this is the entire commit message for the two patches? Based on the note above, it seems like you could *at least* reference the UEFI 2.5 spec in the commit message. After looking through the patches, I figured out that these patches were for SecurityPkg. Instead, I think you should prefix the subject line (the first line) of your commit message with the package. See: https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format You should Cc Chao Zhang, who is listed as the SecurityPkg owner in Maintainers.txt. Is this module entirely written from scratch, or is there some previously existing EDK II code that reviewers might be able to compare it with? -Jordan ------------------------------------------------------------------------------ One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight. http://ad.doubleclick.net/ddm/clk/290420510;117567292;y _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight. http://ad.doubleclick.net/ddm/clk/290420510;117567292;y _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
