[PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[Jayanth.Raghuram]

[-- 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

Re: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[Liming Gao]

[-- 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 --]

Re: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[Jayanth.Raghuram]

[-- 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 --]

Re: [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[Liming Gao]

[-- 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 --]

Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[Ni, Ray]

[-- 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 --]

Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[Jayanth.Raghuram]

[-- 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 --]

Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[Leif Lindholm]

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
> 
> 

Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[Laszlo Ersek]

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

Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[Liming Gao]

[-- 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 --]

Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[Ni, Ray]

> -----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.

Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[Nate DeSimone]

[-- 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 --]

Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[Laszlo Ersek]

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

Re: [edk2-devel] [PATCH] MdePkg/DxeHstiLib: Added checks to improve error handling.

[Laszlo Ersek]

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