Hello Jeshua,

On 10/6/23 18:28, Jeshua Smith wrote:
This patch enhances error handling and reporting in the CM ObjectParser.
Specifically:
1. ObjectIDs used as array indexes are checked for being out of bounds,
    and if so an error message is printed before the assert.
2. An error message is printed for unsupported NameSpaceIDs.
3. Adds support for unimplemented parsers by allowing IDs to list a
    NULL parser, resulting in an unimplemented message being printed.

I am not sure I see in which context 3. would be used/necessary. Is it possible
to detail ?

(Code-wise everything looks good to me)

Regards,
Pierre



Signed-off-by: Jeshua Smith <jesh...@nvidia.com>
---
  .../ConfigurationManagerObjectParser.c        | 47 +++++++++++++------
  1 file changed, 33 insertions(+), 14 deletions(-)

diff --git 
a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
 
b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
index 92df1efee8..22b8fdb906 100644
--- 
a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
+++ 
b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
@@ -795,6 +795,7 @@ STATIC CONST CM_OBJ_PARSER_ARRAY  
StdNamespaceObjectParser[] = {
      ARRAY_SIZE (StdObjAcpiTableInfoParser) },
    { "EStdObjSmbiosTableList", StdObjSmbiosTableInfoParser,
      ARRAY_SIZE (StdObjSmbiosTableInfoParser) },
+  { "EStdObjMax",             NULL,                       0}
  };
/** Print string data.
@@ -1066,6 +1067,12 @@ ParseCmObjDesc (
          return;
        }
+ if (ObjId >= ARRAY_SIZE (StdNamespaceObjectParser)) {
+        DEBUG ((DEBUG_ERROR, "ObjId 0x%x is missing from the 
StdNamespaceObjectParser array\n", ObjId));
+        ASSERT (0);
+        return;
+      }
+
        ParserArray = &StdNamespaceObjectParser[ObjId];
        break;
      case EObjNameSpaceArm:
@@ -1074,10 +1081,17 @@ ParseCmObjDesc (
          return;
        }
+ if (ObjId >= ARRAY_SIZE (ArmNamespaceObjectParser)) {
+        DEBUG ((DEBUG_ERROR, "ObjId 0x%x is missing from the 
ArmNamespaceObjectParser array\n", ObjId));
+        ASSERT (0);
+        return;
+      }
+
        ParserArray = &ArmNamespaceObjectParser[ObjId];
        break;
      default:
        // Not supported
+      DEBUG ((DEBUG_ERROR, "NameSpaceId 0x%x, ObjId 0x%x is not supported by the 
parser\n", NameSpaceId, ObjId));
        ASSERT (0);
        return;
    } // switch
@@ -1095,21 +1109,26 @@ ParseCmObjDesc (
        ObjIndex + 1,
        ObjectCount
        ));
-    PrintCmObjDesc (
-      (VOID *)((UINTN)CmObjDesc->Data + Offset),
-      ParserArray->Parser,
-      ParserArray->ItemCount,
-      &RemainingSize,
-      1
-      );
-    if ((RemainingSize > CmObjDesc->Size) ||
-        (RemainingSize < 0))
-    {
-      ASSERT (0);
-      return;
-    }
+    if (ParserArray->Parser == NULL) {
+      DEBUG ((DEBUG_ERROR, "Parser not implemented\n"));
+      RemainingSize = 0;
+    } else {
+      PrintCmObjDesc (
+        (VOID *)((UINTN)CmObjDesc->Data + Offset),
+        ParserArray->Parser,
+        ParserArray->ItemCount,
+        &RemainingSize,
+        1
+        );
+      if ((RemainingSize > CmObjDesc->Size) ||
+          (RemainingSize < 0))
+      {
+        ASSERT (0);
+        return;
+      }
- Offset = CmObjDesc->Size - RemainingSize;
+      Offset = CmObjDesc->Size - RemainingSize;
+    }
    } // for
ASSERT (RemainingSize == 0);


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109486): https://edk2.groups.io/g/devel/message/109486
Mute This Topic: https://groups.io/mt/101801385/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to