* [PATCH 0/2] Add error handling and initialize variables
@ 2019-09-02 12:30 Zhang, Shenglei
2019-09-02 12:30 ` [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei
2019-09-02 12:30 ` [PATCH 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei
0 siblings, 2 replies; 6+ messages in thread
From: Zhang, Shenglei @ 2019-09-02 12:30 UTC (permalink / raw)
To: devel; +Cc: Michael Kubacki, Chasel Chiu, Nate DeSimone, Liming Gao
1.Add error handling to enhance status checking.
2.Initialize the variables before used and add check before
FreePool().
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Shenglei Zhang (2):
MinPlatformPkg/AcpiTables: Initialize variables before used
MinPlatformPkg/AcpiTables: Add error handling to
SortCpuLocalApicInTable
.../Acpi/AcpiTables/AcpiPlatform.c | 27 ++++++++++++-------
1 file changed, 17 insertions(+), 10 deletions(-)
--
2.18.0.windows.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used
2019-09-02 12:30 [PATCH 0/2] Add error handling and initialize variables Zhang, Shenglei
@ 2019-09-02 12:30 ` Zhang, Shenglei
2019-09-02 13:48 ` Chiu, Chasel
2019-09-10 22:28 ` Nate DeSimone
2019-09-02 12:30 ` [PATCH 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei
1 sibling, 2 replies; 6+ messages in thread
From: Zhang, Shenglei @ 2019-09-02 12:30 UTC (permalink / raw)
To: devel; +Cc: Michael Kubacki, Chasel Chiu, Nate DeSimone, Liming Gao
MadtStructs, NewMadtTable and MaxMadtStructCount are not initialized
before used or at the proper place. So assign values to them at the
beginning and change the logic when freeing MadtStructs and
NewMadtTable.
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
.../Acpi/AcpiTables/AcpiPlatform.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 5eb72792..85d1bd9a 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -867,13 +867,15 @@ InstallMadtFromScratch (
UINT32 PcIoApicMask;
UINTN PcIoApicIndex;
+ MadtStructs = NULL;
+ NewMadtTable = NULL;
+ MaxMadtStructCount = 0;
+
DetectApicIdMap();
// Call for Local APIC ID Reorder
SortCpuLocalApicInTable ();
- NewMadtTable = NULL;
-
MaxMadtStructCount = (UINT32) (
MAX_CPU_NUM + // processor local APIC structures
MAX_CPU_NUM + // processor local x2APIC structures
@@ -1115,14 +1117,15 @@ Done:
//
// Free memory
//
- for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount; MadtStructsIndex++) {
- if (MadtStructs[MadtStructsIndex] != NULL) {
- FreePool (MadtStructs[MadtStructsIndex]);
+ if (MadtStructs != NULL){
+ for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount; MadtStructsIndex++) {
+ if (MadtStructs[MadtStructsIndex] != NULL) {
+ FreePool (MadtStructs[MadtStructsIndex]);
+ }
}
+ FreePool (MadtStructs);
}
-
- FreePool (MadtStructs);
-
+
if (NewMadtTable != NULL) {
FreePool (NewMadtTable);
}
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable
2019-09-02 12:30 [PATCH 0/2] Add error handling and initialize variables Zhang, Shenglei
2019-09-02 12:30 ` [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei
@ 2019-09-02 12:30 ` Zhang, Shenglei
2019-09-02 13:54 ` Chiu, Chasel
1 sibling, 1 reply; 6+ messages in thread
From: Zhang, Shenglei @ 2019-09-02 12:30 UTC (permalink / raw)
To: devel; +Cc: Michael Kubacki, Chasel Chiu, Nate DeSimone, Liming Gao
Change ASSERT_EFI_ERROR to return value when the "if" statement is true.
As a result, when SortCpuLocalApicInTable is called, error handling is
needed. So add "if" statement to judge the returned Status from
SortCpuLocalApicInTable () in function InstallMadtFromScratch().
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
.../Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 85d1bd9a..dc68dfaa 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -354,7 +354,7 @@ SortCpuLocalApicInTable (
if(MAX_CPU_NUM <= Index) {
DEBUG ((EFI_D_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n"));
- ASSERT_EFI_ERROR(EFI_INVALID_PARAMETER);
+ return EFI_INVALID_PARAMETER;
}
TempVal = mCpuApicIdOrderTable[Index].ApicId;
@@ -874,7 +874,11 @@ InstallMadtFromScratch (
DetectApicIdMap();
// Call for Local APIC ID Reorder
- SortCpuLocalApicInTable ();
+ Status = SortCpuLocalApicInTable ();
+ if (EFI_ERROR (Status)) {
+ DEBUG ((EFI_D_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
+ goto Done;
+ }
MaxMadtStructCount = (UINT32) (
MAX_CPU_NUM + // processor local APIC structures
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used
2019-09-02 12:30 ` [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei
@ 2019-09-02 13:48 ` Chiu, Chasel
2019-09-10 22:28 ` Nate DeSimone
1 sibling, 0 replies; 6+ messages in thread
From: Chiu, Chasel @ 2019-09-02 13:48 UTC (permalink / raw)
To: Zhang, Shenglei, devel@edk2.groups.io
Cc: Kubacki, Michael A, Desimone, Nathaniel L, Gao, Liming
Please extend copyright. With this update, Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
> -----Original Message-----
> From: Zhang, Shenglei
> Sent: Monday, September 2, 2019 8:30 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before
> used
>
> MadtStructs, NewMadtTable and MaxMadtStructCount are not initialized
> before used or at the proper place. So assign values to them at the
> beginning and change the logic when freeing MadtStructs and
> NewMadtTable.
>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
> .../Acpi/AcpiTables/AcpiPlatform.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 5eb72792..85d1bd9a 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -867,13 +867,15 @@ InstallMadtFromScratch (
> UINT32
> PcIoApicMask;
> UINTN
> PcIoApicIndex;
>
> + MadtStructs = NULL;
> + NewMadtTable = NULL;
> + MaxMadtStructCount = 0;
> +
> DetectApicIdMap();
>
> // Call for Local APIC ID Reorder
> SortCpuLocalApicInTable ();
>
> - NewMadtTable = NULL;
> -
> MaxMadtStructCount = (UINT32) (
> MAX_CPU_NUM + // processor local APIC structures
> MAX_CPU_NUM + // processor local x2APIC structures
> @@ -1115,14 +1117,15 @@ Done:
> //
> // Free memory
> //
> - for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount;
> MadtStructsIndex++) {
> - if (MadtStructs[MadtStructsIndex] != NULL) {
> - FreePool (MadtStructs[MadtStructsIndex]);
> + if (MadtStructs != NULL){
> + for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount;
> MadtStructsIndex++) {
> + if (MadtStructs[MadtStructsIndex] != NULL) {
> + FreePool (MadtStructs[MadtStructsIndex]);
> + }
> }
> + FreePool (MadtStructs);
> }
> -
> - FreePool (MadtStructs);
> -
> +
> if (NewMadtTable != NULL) {
> FreePool (NewMadtTable);
> }
> --
> 2.18.0.windows.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable
2019-09-02 12:30 ` [PATCH 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei
@ 2019-09-02 13:54 ` Chiu, Chasel
0 siblings, 0 replies; 6+ messages in thread
From: Chiu, Chasel @ 2019-09-02 13:54 UTC (permalink / raw)
To: Zhang, Shenglei, devel@edk2.groups.io
Cc: Kubacki, Michael A, Desimone, Nathaniel L, Gao, Liming
Same with 1/2 patch, please extend copyright.
With above update, Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
> -----Original Message-----
> From: Zhang, Shenglei
> Sent: Monday, September 2, 2019 8:30 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [PATCH 2/2] MinPlatformPkg/AcpiTables: Add error handling to
> SortCpuLocalApicInTable
>
> Change ASSERT_EFI_ERROR to return value when the "if" statement is true.
> As a result, when SortCpuLocalApicInTable is called, error handling is
> needed. So add "if" statement to judge the returned Status from
> SortCpuLocalApicInTable () in function InstallMadtFromScratch().
>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
> .../Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 85d1bd9a..dc68dfaa 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -354,7 +354,7 @@ SortCpuLocalApicInTable (
>
> if(MAX_CPU_NUM <= Index) {
> DEBUG ((EFI_D_ERROR, "Asserting the SortCpuLocalApicInTable
> Index Bufferflow\n"));
> - ASSERT_EFI_ERROR(EFI_INVALID_PARAMETER);
> + return EFI_INVALID_PARAMETER;
> }
>
> TempVal = mCpuApicIdOrderTable[Index].ApicId;
> @@ -874,7 +874,11 @@ InstallMadtFromScratch (
> DetectApicIdMap();
>
> // Call for Local APIC ID Reorder
> - SortCpuLocalApicInTable ();
> + Status = SortCpuLocalApicInTable ();
> + if (EFI_ERROR (Status)) {
> + DEBUG ((EFI_D_ERROR, "SortCpuLocalApicInTable failed: %r\n",
> Status));
> + goto Done;
> + }
>
> MaxMadtStructCount = (UINT32) (
> MAX_CPU_NUM + // processor local APIC structures
> --
> 2.18.0.windows.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used
2019-09-02 12:30 ` [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei
2019-09-02 13:48 ` Chiu, Chasel
@ 2019-09-10 22:28 ` Nate DeSimone
1 sibling, 0 replies; 6+ messages in thread
From: Nate DeSimone @ 2019-09-10 22:28 UTC (permalink / raw)
To: Zhang, Shenglei, devel@edk2.groups.io
Cc: Kubacki, Michael A, Chiu, Chasel, Gao, Liming
Hi Shenglei,
Please send a second patch series with the copyrights updated.
Thanks,
Nate
-----Original Message-----
From: Zhang, Shenglei
Sent: Monday, September 2, 2019 5:30 AM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used
MadtStructs, NewMadtTable and MaxMadtStructCount are not initialized before used or at the proper place. So assign values to them at the beginning and change the logic when freeing MadtStructs and NewMadtTable.
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
.../Acpi/AcpiTables/AcpiPlatform.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 5eb72792..85d1bd9a 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -867,13 +867,15 @@ InstallMadtFromScratch (
UINT32 PcIoApicMask;
UINTN PcIoApicIndex;
+ MadtStructs = NULL;
+ NewMadtTable = NULL;
+ MaxMadtStructCount = 0;
+
DetectApicIdMap();
// Call for Local APIC ID Reorder
SortCpuLocalApicInTable ();
- NewMadtTable = NULL;
-
MaxMadtStructCount = (UINT32) (
MAX_CPU_NUM + // processor local APIC structures
MAX_CPU_NUM + // processor local x2APIC structures
@@ -1115,14 +1117,15 @@ Done:
//
// Free memory
//
- for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount; MadtStructsIndex++) {
- if (MadtStructs[MadtStructsIndex] != NULL) {
- FreePool (MadtStructs[MadtStructsIndex]);
+ if (MadtStructs != NULL){
+ for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount; MadtStructsIndex++) {
+ if (MadtStructs[MadtStructsIndex] != NULL) {
+ FreePool (MadtStructs[MadtStructsIndex]);
+ }
}
+ FreePool (MadtStructs);
}
-
- FreePool (MadtStructs);
-
+
if (NewMadtTable != NULL) {
FreePool (NewMadtTable);
}
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-10 22:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-02 12:30 [PATCH 0/2] Add error handling and initialize variables Zhang, Shenglei
2019-09-02 12:30 ` [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei
2019-09-02 13:48 ` Chiu, Chasel
2019-09-10 22:28 ` Nate DeSimone
2019-09-02 12:30 ` [PATCH 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei
2019-09-02 13:54 ` Chiu, Chasel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox