* [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. @ 2019-08-26 19:54 Jayanth.Raghuram 2019-08-27 1:56 ` Liming Gao 0 siblings, 1 reply; 13+ messages in thread From: Jayanth.Raghuram @ 2019-08-26 19:54 UTC (permalink / raw) To: devel; +Cc: Wei.G.Liu [-- Attachment #1.1: Type: text/plain, Size: 672 bytes --] Subject: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Added checks for return parameters of LocateHandleBuffer & GetSupportedTypes function calls in InternalHstiFindAip to improve error handling. An issue was observed on Dell Poweredge R740, where the Dell PERC H740P controller UEFI driver returned InfoTypesBuffer = NULL, InfoTypesBufferCount = 0 and caused an FreePool assert. Signed-off-by: Jayanth Raghuram <Jayanth.Raghuram@Dell.com> Cc: Wei G Liu <Wei_G_Liu@Dell.com> Attached: 0001-MdePkg-DxeHstiLib-Added-checks-to-improve-error-hand.patch Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 [-- Attachment #1.2: Type: text/html, Size: 4057 bytes --] [-- Attachment #2: 0001-MdePkg-DxeHstiLib-Added-checks-to-improve-error-hand.patch --] [-- Type: application/octet-stream, Size: 1643 bytes --] From 3006d06fe462fc2a18e872b8b69001d8a74bbb93 Mon Sep 17 00:00:00 2001 Message-Id: <3006d06fe462fc2a18e872b8b69001d8a74bbb93.1566848730.git.Jayanth_Raghuram@Dell.com> From: Jayanth Raghuram <Jayanth_Raghuram@Dell.com> Date: Mon, 26 Aug 2019 13:33:52 -0500 Subject: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Added checks for return parameters of LocateHandleBuffer & GetSupportedTypes function calls in InternalHstiFindAip to improve error handling. An issue was observed on Dell Poweredge R740, where the Dell PERC H740P controller UEFI driver returned InfoTypesBuffer = NULL, InfoTypesBufferCount = 0 and caused an FreePool assert. Signed-off-by: Jayanth Raghuram <Jayanth.Raghuram@Dell.com> Cc: Wei G Liu <Wei_G_Liu@Dell.com> --- MdePkg/Library/DxeHstiLib/HstiDxe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MdePkg/Library/DxeHstiLib/HstiDxe.c b/MdePkg/Library/DxeHstiLib/HstiDxe.c index 9466e3dcb3..104e9d4aab 100644 --- a/MdePkg/Library/DxeHstiLib/HstiDxe.c +++ b/MdePkg/Library/DxeHstiLib/HstiDxe.c @@ -51,7 +51,7 @@ InternalHstiFindAip ( &NoHandles, &Handles ); - if (EFI_ERROR (Status)) { + if (EFI_ERROR (Status) || (Handles == NULL) || (NoHandles == 0)) { return NULL; } @@ -77,7 +77,7 @@ InternalHstiFindAip ( &InfoTypesBuffer, &InfoTypesBufferCount ); - if (EFI_ERROR (Status)) { + if (EFI_ERROR (Status) || (InfoTypesBuffer == NULL) || (InfoTypesBufferCount == 0)) { continue; } -- 2.19.1.windows.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. 2019-08-26 19:54 [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling Jayanth.Raghuram @ 2019-08-27 1:56 ` Liming Gao 2019-08-27 18:29 ` Jayanth.Raghuram 0 siblings, 1 reply; 13+ messages in thread From: Liming Gao @ 2019-08-27 1:56 UTC (permalink / raw) To: devel@edk2.groups.io, Jayanth.Raghuram@dell.com; +Cc: Wei.G.Liu@dell.com [-- Attachment #1: Type: text/plain, Size: 1283 bytes --] Can you follow this process to send this patch again? https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process And, the commit message format is https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format Thanks Liming From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Jayanth.Raghuram@dell.com Sent: Tuesday, August 27, 2019 3:55 AM To: devel@edk2.groups.io Cc: Wei.G.Liu@dell.com Subject: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Subject: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Added checks for return parameters of LocateHandleBuffer & GetSupportedTypes function calls in InternalHstiFindAip to improve error handling. An issue was observed on Dell Poweredge R740, where the Dell PERC H740P controller UEFI driver returned InfoTypesBuffer = NULL, InfoTypesBufferCount = 0 and caused an FreePool assert. Signed-off-by: Jayanth Raghuram <Jayanth.Raghuram@Dell.com<mailto:Jayanth.Raghuram@Dell.com>> Cc: Wei G Liu <Wei_G_Liu@Dell.com<mailto:Wei_G_Liu@Dell.com>> Attached: 0001-MdePkg-DxeHstiLib-Added-checks-to-improve-error-hand.patch Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 [-- Attachment #2: Type: text/html, Size: 6256 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. 2019-08-27 1:56 ` Liming Gao @ 2019-08-27 18:29 ` Jayanth.Raghuram 2019-08-28 7:59 ` Liming Gao 0 siblings, 1 reply; 13+ messages in thread From: Jayanth.Raghuram @ 2019-08-27 18:29 UTC (permalink / raw) To: liming.gao, devel; +Cc: Wei.G.Liu [-- Attachment #1: Type: text/plain, Size: 1995 bytes --] Hi Liming, I sent the review request based on the description in the links that you mentioned below. I sent it in an Dell Email since I cannot use GIT SMTP to send email out from our servers. Please let me know what is wrong and I can help rectify that. Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 From: Gao, Liming <liming.gao@intel.com> Sent: Monday, August 26, 2019 8:57 PM To: devel@edk2.groups.io; Raghuram, Jayanth Cc: Liu, Wei G Subject: RE: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. [EXTERNAL EMAIL] Can you follow this process to send this patch again? https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process And, the commit message format is https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format Thanks Liming From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of Jayanth.Raghuram@dell.com<mailto:Jayanth.Raghuram@dell.com> Sent: Tuesday, August 27, 2019 3:55 AM To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Wei.G.Liu@dell.com<mailto:Wei.G.Liu@dell.com> Subject: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Subject: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Added checks for return parameters of LocateHandleBuffer & GetSupportedTypes function calls in InternalHstiFindAip to improve error handling. An issue was observed on Dell Poweredge R740, where the Dell PERC H740P controller UEFI driver returned InfoTypesBuffer = NULL, InfoTypesBufferCount = 0 and caused an FreePool assert. Signed-off-by: Jayanth Raghuram <Jayanth.Raghuram@Dell.com<mailto:Jayanth.Raghuram@Dell.com>> Cc: Wei G Liu <Wei_G_Liu@Dell.com<mailto:Wei_G_Liu@Dell.com>> Attached: 0001-MdePkg-DxeHstiLib-Added-checks-to-improve-error-hand.patch Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 [-- Attachment #2: Type: text/html, Size: 8976 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. 2019-08-27 18:29 ` Jayanth.Raghuram @ 2019-08-28 7:59 ` Liming Gao 2019-08-28 17:33 ` [edk2-devel] " Ni, Ray 0 siblings, 1 reply; 13+ messages in thread From: Liming Gao @ 2019-08-28 7:59 UTC (permalink / raw) To: Jayanth.Raghuram@dell.com, devel@edk2.groups.io; +Cc: Wei.G.Liu@dell.com [-- Attachment #1: Type: text/plain, Size: 2769 bytes --] OK. So, you can't use git send-email to send this patch. Another way is to fork edk2 and create the branch to include this change. Then, send the mail to let people review this patch in your branch. And, for this patch, can you submit BZ https://bugzilla.tianocore.org/ first? Then, update its commit message format based on this wiki. https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format Thanks Liming From: Jayanth.Raghuram@dell.com [mailto:Jayanth.Raghuram@dell.com] Sent: Wednesday, August 28, 2019 2:30 AM To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io Cc: Wei.G.Liu@dell.com Subject: RE: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Hi Liming, I sent the review request based on the description in the links that you mentioned below. I sent it in an Dell Email since I cannot use GIT SMTP to send email out from our servers. Please let me know what is wrong and I can help rectify that. Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 From: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> Sent: Monday, August 26, 2019 8:57 PM To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Raghuram, Jayanth Cc: Liu, Wei G Subject: RE: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. [EXTERNAL EMAIL] Can you follow this process to send this patch again? https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process And, the commit message format is https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format Thanks Liming From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of Jayanth.Raghuram@dell.com<mailto:Jayanth.Raghuram@dell.com> Sent: Tuesday, August 27, 2019 3:55 AM To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Wei.G.Liu@dell.com<mailto:Wei.G.Liu@dell.com> Subject: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Subject: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Added checks for return parameters of LocateHandleBuffer & GetSupportedTypes function calls in InternalHstiFindAip to improve error handling. An issue was observed on Dell Poweredge R740, where the Dell PERC H740P controller UEFI driver returned InfoTypesBuffer = NULL, InfoTypesBufferCount = 0 and caused an FreePool assert. Signed-off-by: Jayanth Raghuram <Jayanth.Raghuram@Dell.com<mailto:Jayanth.Raghuram@Dell.com>> Cc: Wei G Liu <Wei_G_Liu@Dell.com<mailto:Wei_G_Liu@Dell.com>> Attached: 0001-MdePkg-DxeHstiLib-Added-checks-to-improve-error-hand.patch Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 [-- Attachment #2: Type: text/html, Size: 11333 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. 2019-08-28 7:59 ` Liming Gao @ 2019-08-28 17:33 ` Ni, Ray 2019-08-29 1:19 ` Jayanth.Raghuram 2019-08-29 9:49 ` Leif Lindholm 0 siblings, 2 replies; 13+ messages in thread From: Ni, Ray @ 2019-08-28 17:33 UTC (permalink / raw) To: Gao, Liming, Cetola, Stephano, Kinney, Michael D, leif.lindholm@linaro.org, Jayanth.Raghuram@dell.com, 'Andrew Fish (afish@apple.com)', Laszlo Ersek Cc: Wei.G.Liu@dell.com, devel@edk2.groups.io [-- Attachment #1: Type: text/plain, Size: 3582 bytes --] Liming, Stephano and all stewards, My understanding is the requirement of embedding patch into the mail body is due to a limitation in old system (01.org). That system couldn't support mail attachments. With the existence of mail attachments capability in new groups.io system, can we accept such kind of patch submission? Or any side effect you see if allowing mail attachments? Thanks, Ray From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Liming Gao Sent: Wednesday, August 28, 2019 12:59 AM To: Jayanth.Raghuram@dell.com; devel@edk2.groups.io Cc: Wei.G.Liu@dell.com Subject: Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. OK. So, you can't use git send-email to send this patch. Another way is to fork edk2 and create the branch to include this change. Then, send the mail to let people review this patch in your branch. And, for this patch, can you submit BZ https://bugzilla.tianocore.org/ first? Then, update its commit message format based on this wiki. https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format Thanks Liming From: Jayanth.Raghuram@dell.com<mailto:Jayanth.Raghuram@dell.com> [mailto:Jayanth.Raghuram@dell.com] Sent: Wednesday, August 28, 2019 2:30 AM To: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Wei.G.Liu@dell.com<mailto:Wei.G.Liu@dell.com> Subject: RE: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Hi Liming, I sent the review request based on the description in the links that you mentioned below. I sent it in an Dell Email since I cannot use GIT SMTP to send email out from our servers. Please let me know what is wrong and I can help rectify that. Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 From: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> Sent: Monday, August 26, 2019 8:57 PM To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Raghuram, Jayanth Cc: Liu, Wei G Subject: RE: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. [EXTERNAL EMAIL] Can you follow this process to send this patch again? https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process And, the commit message format is https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format Thanks Liming From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of Jayanth.Raghuram@dell.com<mailto:Jayanth.Raghuram@dell.com> Sent: Tuesday, August 27, 2019 3:55 AM To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Wei.G.Liu@dell.com<mailto:Wei.G.Liu@dell.com> Subject: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Subject: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Added checks for return parameters of LocateHandleBuffer & GetSupportedTypes function calls in InternalHstiFindAip to improve error handling. An issue was observed on Dell Poweredge R740, where the Dell PERC H740P controller UEFI driver returned InfoTypesBuffer = NULL, InfoTypesBufferCount = 0 and caused an FreePool assert. Signed-off-by: Jayanth Raghuram <Jayanth.Raghuram@Dell.com<mailto:Jayanth.Raghuram@Dell.com>> Cc: Wei G Liu <Wei_G_Liu@Dell.com<mailto:Wei_G_Liu@Dell.com>> Attached: 0001-MdePkg-DxeHstiLib-Added-checks-to-improve-error-hand.patch Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 [-- Attachment #2: Type: text/html, Size: 12796 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. 2019-08-28 17:33 ` [edk2-devel] " Ni, Ray @ 2019-08-29 1:19 ` Jayanth.Raghuram 2019-08-29 14:28 ` Liming Gao 2019-08-29 9:49 ` Leif Lindholm 1 sibling, 1 reply; 13+ messages in thread From: Jayanth.Raghuram @ 2019-08-29 1:19 UTC (permalink / raw) To: ray.ni, liming.gao, stephano.cetola, michael.d.kinney, leif.lindholm, afish, lersek Cc: Wei.G.Liu, devel [-- Attachment #1: Type: text/plain, Size: 4574 bytes --] Hi Liming, Per your request, I have created an pull request as below: https://github.com/tianocore/edk2/pull/147 MdePkg/DxeHstiLib: Added checks to improve error handling. #147 Open JayRaghuram<https://github.com/JayRaghuram> wants to merge 1 commit into tianocore:master<https://github.com/tianocore/edk2> from JayRaghuram:master<https://github.com/JayRaghuram/edk2> Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 From: Ni, Ray <ray.ni@intel.com> Sent: Wednesday, August 28, 2019 12:33 PM To: Gao, Liming; Cetola, Stephano; Kinney, Michael D; leif.lindholm@linaro.org; Raghuram, Jayanth; 'Andrew Fish (afish@apple.com)'; Laszlo Ersek Cc: Liu, Wei G; devel@edk2.groups.io Subject: RE: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. [EXTERNAL EMAIL] Liming, Stephano and all stewards, My understanding is the requirement of embedding patch into the mail body is due to a limitation in old system (01.org). That system couldn't support mail attachments. With the existence of mail attachments capability in new groups.io system, can we accept such kind of patch submission? Or any side effect you see if allowing mail attachments? Thanks, Ray From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Liming Gao Sent: Wednesday, August 28, 2019 12:59 AM To: Jayanth.Raghuram@dell.com<mailto:Jayanth.Raghuram@dell.com>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Wei.G.Liu@dell.com<mailto:Wei.G.Liu@dell.com> Subject: Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. OK. So, you can't use git send-email to send this patch. Another way is to fork edk2 and create the branch to include this change. Then, send the mail to let people review this patch in your branch. And, for this patch, can you submit BZ https://bugzilla.tianocore.org/ first? Then, update its commit message format based on this wiki. https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format Thanks Liming From: Jayanth.Raghuram@dell.com<mailto:Jayanth.Raghuram@dell.com> [mailto:Jayanth.Raghuram@dell.com] Sent: Wednesday, August 28, 2019 2:30 AM To: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Wei.G.Liu@dell.com<mailto:Wei.G.Liu@dell.com> Subject: RE: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Hi Liming, I sent the review request based on the description in the links that you mentioned below. I sent it in an Dell Email since I cannot use GIT SMTP to send email out from our servers. Please let me know what is wrong and I can help rectify that. Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 From: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> Sent: Monday, August 26, 2019 8:57 PM To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Raghuram, Jayanth Cc: Liu, Wei G Subject: RE: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. [EXTERNAL EMAIL] Can you follow this process to send this patch again? https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process And, the commit message format is https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format Thanks Liming From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of Jayanth.Raghuram@dell.com<mailto:Jayanth.Raghuram@dell.com> Sent: Tuesday, August 27, 2019 3:55 AM To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Wei.G.Liu@dell.com<mailto:Wei.G.Liu@dell.com> Subject: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Subject: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Added checks for return parameters of LocateHandleBuffer & GetSupportedTypes function calls in InternalHstiFindAip to improve error handling. An issue was observed on Dell Poweredge R740, where the Dell PERC H740P controller UEFI driver returned InfoTypesBuffer = NULL, InfoTypesBufferCount = 0 and caused an FreePool assert. Signed-off-by: Jayanth Raghuram <Jayanth.Raghuram@Dell.com<mailto:Jayanth.Raghuram@Dell.com>> Cc: Wei G Liu <Wei_G_Liu@Dell.com<mailto:Wei_G_Liu@Dell.com>> Attached: 0001-MdePkg-DxeHstiLib-Added-checks-to-improve-error-hand.patch Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 [-- Attachment #2: Type: text/html, Size: 18571 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. 2019-08-29 1:19 ` Jayanth.Raghuram @ 2019-08-29 14:28 ` Liming Gao 0 siblings, 0 replies; 13+ messages in thread From: Liming Gao @ 2019-08-29 14:28 UTC (permalink / raw) To: Jayanth.Raghuram@dell.com, Ni, Ray, Cetola, Stephano, Kinney, Michael D, leif.lindholm@linaro.org, afish@apple.com, lersek@redhat.com Cc: Wei.G.Liu@dell.com, devel@edk2.groups.io, Gao, Liming [-- Attachment #1: Type: text/plain, Size: 6124 bytes --] Jayanth: Thanks for your update commit message. But, edk2 project doesn't use pull request. You can send https://github.com/JayRaghuram/edk2 and say your patch is in this tree. Leif gives the option. You need copy the patch content into the mail, then send this mail to devel@edk2.groups.io<mailto:devel@edk2.groups.io> for code review. And, for this patch, I have some comments. 1. I have not given my reviewed-by. Please don't add it. 2. LocateHandleBuffer() service. UEFI spec defines that if there are no handles in the handle database that match the search criteria, then EFI_NOT_FOUND is returned. So, when EFI_SUCCESS return, don't need to check Handles and NoHandles. This change is not required. 3. GetSupportedTypes() service. UEFI spec doesn't define EFI_NOT_FOUND. So, when EFI_SUCCESS return, InfoTypesBuffer may be NULL or InfoTypesBufferCount may be zero. I agree to add this checker. Thanks Liming From: Jayanth.Raghuram@dell.com [mailto:Jayanth.Raghuram@dell.com] Sent: Thursday, August 29, 2019 9:20 AM To: Ni, Ray <ray.ni@intel.com>; Gao, Liming <liming.gao@intel.com>; Cetola, Stephano <stephano.cetola@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; leif.lindholm@linaro.org; afish@apple.com; lersek@redhat.com Cc: Wei.G.Liu@dell.com; devel@edk2.groups.io Subject: RE: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Hi Liming, Per your request, I have created an pull request as below: https://github.com/tianocore/edk2/pull/147 MdePkg/DxeHstiLib: Added checks to improve error handling. #147 Open JayRaghuram<https://github.com/JayRaghuram> wants to merge 1 commit into tianocore:master<https://github.com/tianocore/edk2> from JayRaghuram:master<https://github.com/JayRaghuram/edk2> Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>> Sent: Wednesday, August 28, 2019 12:33 PM To: Gao, Liming; Cetola, Stephano; Kinney, Michael D; leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>; Raghuram, Jayanth; 'Andrew Fish (afish@apple.com<mailto:afish@apple.com>)'; Laszlo Ersek Cc: Liu, Wei G; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Subject: RE: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. [EXTERNAL EMAIL] Liming, Stephano and all stewards, My understanding is the requirement of embedding patch into the mail body is due to a limitation in old system (01.org). That system couldn't support mail attachments. With the existence of mail attachments capability in new groups.io system, can we accept such kind of patch submission? Or any side effect you see if allowing mail attachments? Thanks, Ray From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Liming Gao Sent: Wednesday, August 28, 2019 12:59 AM To: Jayanth.Raghuram@dell.com<mailto:Jayanth.Raghuram@dell.com>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Wei.G.Liu@dell.com<mailto:Wei.G.Liu@dell.com> Subject: Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. OK. So, you can't use git send-email to send this patch. Another way is to fork edk2 and create the branch to include this change. Then, send the mail to let people review this patch in your branch. And, for this patch, can you submit BZ https://bugzilla.tianocore.org/ first? Then, update its commit message format based on this wiki. https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format Thanks Liming From: Jayanth.Raghuram@dell.com<mailto:Jayanth.Raghuram@dell.com> [mailto:Jayanth.Raghuram@dell.com] Sent: Wednesday, August 28, 2019 2:30 AM To: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Wei.G.Liu@dell.com<mailto:Wei.G.Liu@dell.com> Subject: RE: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Hi Liming, I sent the review request based on the description in the links that you mentioned below. I sent it in an Dell Email since I cannot use GIT SMTP to send email out from our servers. Please let me know what is wrong and I can help rectify that. Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 From: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> Sent: Monday, August 26, 2019 8:57 PM To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Raghuram, Jayanth Cc: Liu, Wei G Subject: RE: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. [EXTERNAL EMAIL] Can you follow this process to send this patch again? https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process And, the commit message format is https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format Thanks Liming From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of Jayanth.Raghuram@dell.com<mailto:Jayanth.Raghuram@dell.com> Sent: Tuesday, August 27, 2019 3:55 AM To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Wei.G.Liu@dell.com<mailto:Wei.G.Liu@dell.com> Subject: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Subject: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. Added checks for return parameters of LocateHandleBuffer & GetSupportedTypes function calls in InternalHstiFindAip to improve error handling. An issue was observed on Dell Poweredge R740, where the Dell PERC H740P controller UEFI driver returned InfoTypesBuffer = NULL, InfoTypesBufferCount = 0 and caused an FreePool assert. Signed-off-by: Jayanth Raghuram <Jayanth.Raghuram@Dell.com<mailto:Jayanth.Raghuram@Dell.com>> Cc: Wei G Liu <Wei_G_Liu@Dell.com<mailto:Wei_G_Liu@Dell.com>> Attached: 0001-MdePkg-DxeHstiLib-Added-checks-to-improve-error-hand.patch Regards Jayanth Raghuram DellEMC | Server Platform BIOS office + 1 512 723 1470 [-- Attachment #2: Type: text/html, Size: 28173 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. 2019-08-28 17:33 ` [edk2-devel] " Ni, Ray 2019-08-29 1:19 ` Jayanth.Raghuram @ 2019-08-29 9:49 ` Leif Lindholm 2019-08-29 14:24 ` Laszlo Ersek 1 sibling, 1 reply; 13+ messages in thread From: Leif Lindholm @ 2019-08-29 9:49 UTC (permalink / raw) To: Ni, Ray Cc: Gao, Liming, Cetola, Stephano, Kinney, Michael D, Jayanth.Raghuram@dell.com, 'Andrew Fish (afish@apple.com)', Laszlo Ersek, Wei.G.Liu@dell.com, devel@edk2.groups.io On Wed, Aug 28, 2019 at 05:33:28PM +0000, Ni, Ray wrote: > Liming, Stephano and all stewards, > My understanding is the requirement of embedding patch into the mail > body is due to a limitation in old system (01.org). That system > couldn't support mail attachments. Oh, it could. I think it was just disabled. > With the existence of mail attachments capability in new groups.io > system, can we accept such kind of patch submission? Or any side > effect you see if allowing mail attachments? Traditionally, the reason for not wanting patches as attachments is that it complicates doing inline code review as part of the email. If the mail system (let's take a wild guess, Outlook/Exchange?) doesn't corrupt the text *too* badly, I don't have an issue with the patch being sent in the message body *and* being attached so it could actually be applied. Alternatively, one could put the patch in the message body and a link to the patch in a public repo where it can be obtained. Best Regards, Leif > > Thanks, > Ray > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Liming Gao > Sent: Wednesday, August 28, 2019 12:59 AM > To: Jayanth.Raghuram@dell.com; devel@edk2.groups.io > Cc: Wei.G.Liu@dell.com > Subject: Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. > > OK. So, you can't use git send-email to send this patch. Another way > is to fork edk2 and create the branch to include this change. > Then, send the mail to let people review this patch in your branch. > > And, for this patch, can you submit BZ https://bugzilla.tianocore.org/ first? > Then, update its commit message format based on this wiki. > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format > > Thanks > Liming > From: Jayanth.Raghuram@dell.com<mailto:Jayanth.Raghuram@dell.com> [mailto:Jayanth.Raghuram@dell.com] > Sent: Wednesday, August 28, 2019 2:30 AM > To: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> > Cc: Wei.G.Liu@dell.com<mailto:Wei.G.Liu@dell.com> > Subject: RE: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. > > Hi Liming, > > I sent the review request based on the description in the links that you mentioned below. > I sent it in an Dell Email since I cannot use GIT SMTP to send email out from our servers. > Please let me know what is wrong and I can help rectify that. > > Regards > Jayanth Raghuram > DellEMC | Server Platform BIOS > office + 1 512 723 1470 > > From: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> > Sent: Monday, August 26, 2019 8:57 PM > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Raghuram, Jayanth > Cc: Liu, Wei G > Subject: RE: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. > > > [EXTERNAL EMAIL] > Can you follow this process to send this patch again? > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process > > And, the commit message format is > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format > > Thanks > Liming > From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of Jayanth.Raghuram@dell.com<mailto:Jayanth.Raghuram@dell.com> > Sent: Tuesday, August 27, 2019 3:55 AM > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > Cc: Wei.G.Liu@dell.com<mailto:Wei.G.Liu@dell.com> > Subject: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. > > Subject: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. > Added checks for return parameters of LocateHandleBuffer & GetSupportedTypes > function calls in InternalHstiFindAip to improve error handling. An issue was > observed on Dell Poweredge R740, where the Dell PERC H740P controller UEFI > driver returned InfoTypesBuffer = NULL, InfoTypesBufferCount = 0 and caused > an FreePool assert. > > Signed-off-by: Jayanth Raghuram <Jayanth.Raghuram@Dell.com<mailto:Jayanth.Raghuram@Dell.com>> > Cc: Wei G Liu <Wei_G_Liu@Dell.com<mailto:Wei_G_Liu@Dell.com>> > > Attached: 0001-MdePkg-DxeHstiLib-Added-checks-to-improve-error-hand.patch > > Regards > Jayanth Raghuram > DellEMC | Server Platform BIOS > office + 1 512 723 1470 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. 2019-08-29 9:49 ` Leif Lindholm @ 2019-08-29 14:24 ` Laszlo Ersek 2019-08-29 17:02 ` Ni, Ray 0 siblings, 1 reply; 13+ messages in thread From: Laszlo Ersek @ 2019-08-29 14:24 UTC (permalink / raw) To: Leif Lindholm, Ni, Ray Cc: Gao, Liming, Cetola, Stephano, Kinney, Michael D, Jayanth.Raghuram@dell.com, 'Andrew Fish (afish@apple.com)', Wei.G.Liu@dell.com, devel@edk2.groups.io On 08/29/19 11:49, Leif Lindholm wrote: > On Wed, Aug 28, 2019 at 05:33:28PM +0000, Ni, Ray wrote: >> With the existence of mail attachments capability in new groups.io >> system, can we accept such kind of patch submission? Or any side >> effect you see if allowing mail attachments? > > Traditionally, the reason for not wanting patches as attachments is > that it complicates doing inline code review as part of the email. > > If the mail system (let's take a wild guess, Outlook/Exchange?) > doesn't corrupt the text *too* badly, I don't have an issue with the > patch being sent in the message body *and* being attached so it could > actually be applied. > > Alternatively, one could put the patch in the message body and a link > to the patch in a public repo where it can be obtained. I agree (with either alterative), as follows: - The patch must be pasted manually in the message body, or it must be attached. - The patch must be published in a personal git repo, and the email posting needs to reference that repo / branch. - This is only acceptable as an exception to the workflow, and not as the norm. (As long as our official contribution workflow is mailing list based.) - Doesn't work for patch series, only for single patches. - v2, v3 and so iterations must use separate topic branches in the submitter's personal repository. Non-fast-forward pushes to already posted -- hence publicly referenced -- branches are not acceptable. Effectively, once v(n) is posted, the submitter should consider the matching topic branch in their own repo read-only. - The edk2 package maintainer that ends up pushing the patch is responsible for ensuring that the patch taken with "git fetch" for integration is identical to the patch that was manually pasted by the submitter. The idea is that mailing list based review work as always, plus that the patch that goes in ultimately be the same patch that got reviewed on the list. Thanks Laszlo ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. 2019-08-29 14:24 ` Laszlo Ersek @ 2019-08-29 17:02 ` Ni, Ray 2019-08-30 2:21 ` Nate DeSimone 2019-08-30 11:49 ` Laszlo Ersek 0 siblings, 2 replies; 13+ messages in thread From: Ni, Ray @ 2019-08-29 17:02 UTC (permalink / raw) To: devel@edk2.groups.io, lersek@redhat.com, Leif Lindholm Cc: Gao, Liming, Cetola, Stephano, Kinney, Michael D, Jayanth.Raghuram@dell.com, 'Andrew Fish (afish@apple.com)', Wei.G.Liu@dell.com > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek > Sent: Thursday, August 29, 2019 7:24 AM > To: Leif Lindholm <leif.lindholm@linaro.org>; Ni, Ray <ray.ni@intel.com> > Cc: Gao, Liming <liming.gao@intel.com>; Cetola, Stephano <stephano.cetola@intel.com>; Kinney, Michael D > <michael.d.kinney@intel.com>; Jayanth.Raghuram@dell.com; 'Andrew Fish (afish@apple.com)' <afish@apple.com>; > Wei.G.Liu@dell.com; devel@edk2.groups.io > Subject: Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. > > On 08/29/19 11:49, Leif Lindholm wrote: > > On Wed, Aug 28, 2019 at 05:33:28PM +0000, Ni, Ray wrote: > > >> With the existence of mail attachments capability in new groups.io > >> system, can we accept such kind of patch submission? Or any side > >> effect you see if allowing mail attachments? > > > > Traditionally, the reason for not wanting patches as attachments is > > that it complicates doing inline code review as part of the email. > > > > If the mail system (let's take a wild guess, Outlook/Exchange?) > > doesn't corrupt the text *too* badly, I don't have an issue with the > > patch being sent in the message body *and* being attached so it could > > actually be applied. Leif, So, message body should have the patch changes for easy inline review. It's optional to carry a .diff attachment. Is my understanding correct? > > - Doesn't work for patch series, only for single patches. Laszlo, Do you mean attachment is not allowed for a series of patch? Why? A mail can carry multiple attachments. That makes the patch series easy to fetch in my opinion from Outlook. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. 2019-08-29 17:02 ` Ni, Ray @ 2019-08-30 2:21 ` Nate DeSimone 2019-08-30 11:52 ` Laszlo Ersek 2019-08-30 11:49 ` Laszlo Ersek 1 sibling, 1 reply; 13+ messages in thread From: Nate DeSimone @ 2019-08-30 2:21 UTC (permalink / raw) To: devel@edk2.groups.io, Ni, Ray, lersek@redhat.com, Leif Lindholm Cc: Gao, Liming, Cetola, Stephano, Kinney, Michael D, Jayanth.Raghuram@dell.com, 'Andrew Fish (afish@apple.com)', Wei.G.Liu@dell.com [-- Attachment #1.1: Type: text/plain, Size: 4829 bytes --] Hi All, It is possible to use git send-email with an exchange server by using DavMail<http://davmail.sourceforge.net/> as a relay. I would recommend that we encourage those whom work at a company which does not allow outbound SMTP connections to send their edk2 patches using this method. Here is how you configure it: 1. Figure out what your exchange server’s network name is: CTRL + Right Click on the Outlook icon in your system tray, select Connection Status… [cid:image001.png@01D55E9C.2E086970] In the status window there might be a few connections, on my network there was connections to an exchange gateway, and then a single connection to the actual exchange server that hosts my personal mailbox, use that server. [cid:image002.png@01D55E9C.9888F800] 1. Download and install DavMail<https://sourceforge.net/projects/davmail/files/davmail/5.3.1/>. On Windows run the .exe as administrator. 2. It looks like the .exe launchers that come with the Windows version of DavMail and old and intended for Java 7 (which is no longer supported and there are licensing issues.) I was able to run DavMail fine using the RedHat supported OpenJDK 11<https://adoptopenjdk.net/releases.html?variant=openjdk11&jvmVariant=hotspot> binaries by opening a Windows command prompt and running the following commands: C:\>cd "Program Files\DavMail\ C:\Program Files\DavMail>java -jar davmail.jar 1. In the DavMail settings dialog, choose Exchange Protocol = EWS and enter the server name from Step #1 using the following format: https://<server_name>/owa [cid:image003.png@01D55E9E.53F35F80] 1. Run these commands in Git Bash: git config --global sendemail.smtpserver localhost:1025 git config --global sendemail.smtpuser <your_email_address> For example: git config --global sendemail.smtpuser nathaniel.l.desimone@intel.com 1. Git send-email should now work. It will popup a dialog box prompting you for a password. Use your outlook/windows password. Thanks, Nate -----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray Sent: Thursday, August 29, 2019 10:02 AM To: devel@edk2.groups.io; lersek@redhat.com; Leif Lindholm <leif.lindholm@linaro.org> Cc: Gao, Liming <liming.gao@intel.com>; Cetola, Stephano <stephano.cetola@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Jayanth.Raghuram@dell.com; 'Andrew Fish (afish@apple.com)' <afish@apple.com>; Wei.G.Liu@dell.com Subject: Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. > -----Original Message----- > From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Laszlo > Ersek > Sent: Thursday, August 29, 2019 7:24 AM > To: Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>; Ni, Ray > <ray.ni@intel.com<mailto:ray.ni@intel.com>> > Cc: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Cetola, Stephano > <stephano.cetola@intel.com<mailto:stephano.cetola@intel.com>>; Kinney, Michael D > <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Jayanth.Raghuram@dell.com<mailto:Jayanth.Raghuram@dell.com>; 'Andrew Fish > (afish@apple.com<mailto:afish@apple.com>)' <afish@apple.com<mailto:afish@apple.com>>; Wei.G.Liu@dell.com<mailto:Wei.G.Liu@dell.com>; > devel@edk2.groups.io<mailto:devel@edk2.groups.io> > Subject: Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. > > On 08/29/19 11:49, Leif Lindholm wrote: > > On Wed, Aug 28, 2019 at 05:33:28PM +0000, Ni, Ray wrote: > > >> With the existence of mail attachments capability in new groups.io > >> system, can we accept such kind of patch submission? Or any side > >> effect you see if allowing mail attachments? > > > > Traditionally, the reason for not wanting patches as attachments is > > that it complicates doing inline code review as part of the email. > > > > If the mail system (let's take a wild guess, Outlook/Exchange?) > > doesn't corrupt the text *too* badly, I don't have an issue with the > > patch being sent in the message body *and* being attached so it > > could actually be applied. Leif, So, message body should have the patch changes for easy inline review. It's optional to carry a .diff attachment. Is my understanding correct? > > - Doesn't work for patch series, only for single patches. Laszlo, Do you mean attachment is not allowed for a series of patch? Why? A mail can carry multiple attachments. That makes the patch series easy to fetch in my opinion from Outlook. [-- Attachment #1.2: Type: text/html, Size: 13981 bytes --] [-- Attachment #2: image001.png --] [-- Type: image/png, Size: 13035 bytes --] [-- Attachment #3: image002.png --] [-- Type: image/png, Size: 7460 bytes --] [-- Attachment #4: image003.png --] [-- Type: image/png, Size: 15595 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. 2019-08-30 2:21 ` Nate DeSimone @ 2019-08-30 11:52 ` Laszlo Ersek 0 siblings, 0 replies; 13+ messages in thread From: Laszlo Ersek @ 2019-08-30 11:52 UTC (permalink / raw) To: Desimone, Nathaniel L, devel@edk2.groups.io, Ni, Ray, Leif Lindholm Cc: Gao, Liming, Cetola, Stephano, Kinney, Michael D, Jayanth.Raghuram@dell.com, 'Andrew Fish (afish@apple.com)', Wei.G.Liu@dell.com On 08/30/19 04:21, Desimone, Nathaniel L wrote: > It is possible to use git send-email with an exchange server by using > DavMail<http://davmail.sourceforge.net/> as a relay. I would > recommend that we encourage those whom work at a company which does > not allow outbound SMTP connections to send their edk2 patches using > this method. This sounds hugely important and helpful. Can people using Outlook please test this setup and report back? Many thanks, Nate! Laszlo ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. 2019-08-29 17:02 ` Ni, Ray 2019-08-30 2:21 ` Nate DeSimone @ 2019-08-30 11:49 ` Laszlo Ersek 1 sibling, 0 replies; 13+ messages in thread From: Laszlo Ersek @ 2019-08-30 11:49 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io, Leif Lindholm Cc: Gao, Liming, Cetola, Stephano, Kinney, Michael D, Jayanth.Raghuram@dell.com, 'Andrew Fish (afish@apple.com)', Wei.G.Liu@dell.com On 08/29/19 19:02, Ni, Ray wrote: > > >> -----Original Message----- >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek >> Sent: Thursday, August 29, 2019 7:24 AM >> To: Leif Lindholm <leif.lindholm@linaro.org>; Ni, Ray <ray.ni@intel.com> >> Cc: Gao, Liming <liming.gao@intel.com>; Cetola, Stephano <stephano.cetola@intel.com>; Kinney, Michael D >> <michael.d.kinney@intel.com>; Jayanth.Raghuram@dell.com; 'Andrew Fish (afish@apple.com)' <afish@apple.com>; >> Wei.G.Liu@dell.com; devel@edk2.groups.io >> Subject: Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling. >> - Doesn't work for patch series, only for single patches. > Laszlo, > Do you mean attachment is not allowed for a series of patch? That's correct; I mean that. > Why? A mail can carry multiple attachments. That makes the patch series easy to fetch in my opinion from Outlook. The goal is to enable fine-grained, email based review comments. Meaning, a reviewer responds to the original email, whereby the original patch is quoted in full in the response. Then the reviewer inserts comments near the code locations in the patch that need updates. This approach works perfectly when the original email was sent with git-send-email. The approach works tolerably (as an exception to our workflow) when the original email was sent manually, with a single patch pasted or attached. Because, in this case, the review comments can still be made context-sensitively, and the threading structure will be sane. The approach does not work at all when a single email carries multiple patches. Quoting becomes a mess, review comments are a lot more difficult to associate with the targeted patch within the series, and so on. If the contribution is a patch series (i.e., not a single patch), and the submitter is unable to use "git-send-email", then the submitter is responsible for manually establishing the exact same shallow thread structure that git-send-email would. Specifically, the submitter first needs to send a cover letter email, then they need to send each patch individually, with numbered subjects, in direct response to the cover letter email. They can paste or attach one patch per email, until the full series is posted. The email thread must reflect the git commits so that a specific git commit has a matching "entry point" into the mailing list discussion -- i.e., a reviewer interested in a given git commit can find the precisely matching email, and the related *sub*-thread that discusses that particular commit. Email is not merely a dumb carrier for patches where it's OK to drop a single patch-bomb email (let alone a ZIP file) on reviewers. A well-structured email thread is vital to careful review, and for the long-term archival of the discussion. Thanks, Laszlo ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-08-30 11:52 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-08-26 19:54 [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling Jayanth.Raghuram 2019-08-27 1:56 ` Liming Gao 2019-08-27 18:29 ` Jayanth.Raghuram 2019-08-28 7:59 ` Liming Gao 2019-08-28 17:33 ` [edk2-devel] " Ni, Ray 2019-08-29 1:19 ` Jayanth.Raghuram 2019-08-29 14:28 ` Liming Gao 2019-08-29 9:49 ` Leif Lindholm 2019-08-29 14:24 ` Laszlo Ersek 2019-08-29 17:02 ` Ni, Ray 2019-08-30 2:21 ` Nate DeSimone 2019-08-30 11:52 ` Laszlo Ersek 2019-08-30 11:49 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox