From 5b2dcdd03d17e24b09e87b9739ea2967b5e8d588 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?=
 <hermes.belusca-maito@reactos.org>
Date: Fri, 5 Apr 2024 17:05:13 +0200
Subject: [PATCH] [FREELDR] BuildArgvForOsLoader: Add a terminating NULL
 pointer to the Argv vector.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Addendum to commits d05be0da3 and bd451f240.

This is required for POSIX compliance, which the ARC specification obeys.

See Section 2.1.2.2.1 (ANSI X3.159-1989) or 5.1.2.2.1 (ISO/IEC 9899:x)
"Program startup":
```
If they are declared, the parameters to the main function shall obey
the following constraints:
— The value of argc shall be nonnegative.
— argv[argc] shall be a null pointer.
[...]
```

See also https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html
---
 boot/freeldr/freeldr/bootmgr.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/boot/freeldr/freeldr/bootmgr.c b/boot/freeldr/freeldr/bootmgr.c
index 98abf5ed955..1f22c3182db 100644
--- a/boot/freeldr/freeldr/bootmgr.c
+++ b/boot/freeldr/freeldr/bootmgr.c
@@ -100,17 +100,18 @@ GetOSLoadingMethod(
     UNREACHABLE;
 }
 
-/*
- * This function converts the list of key=value options in the given operating
+/**
+ * @brief
+ * This function converts the list of Key=Value options in the given operating
  * system section into an ARC-compatible argument vector, providing in addition
  * the extra mandatory Software Loading Environment Variables, following the
  * ARC specification.
- */
+ **/
 static PCHAR*
 BuildArgvForOsLoader(
-    IN PCSTR LoadIdentifier,
-    IN ULONG_PTR SectionId,
-    OUT PULONG pArgc)
+    _In_ PCSTR LoadIdentifier,
+    _In_ ULONG_PTR SectionId,
+    _Out_ PULONG pArgc)
 {
     SIZE_T Size;
     ULONG Count;
@@ -124,7 +125,7 @@ BuildArgvForOsLoader(
 
     ASSERT(SectionId != 0);
 
-    /* Validate the LoadIdentifier (to make tests simpler later) */
+    /* Normalize LoadIdentifier to make subsequent tests simpler */
     if (LoadIdentifier && !*LoadIdentifier)
         LoadIdentifier = NULL;
 
@@ -134,19 +135,24 @@ BuildArgvForOsLoader(
     /*
      * The argument vector contains the program name, the SystemPartition,
      * the LoadIdentifier (optional), and the items in the OS section.
+     * For POSIX compliance, a terminating NULL pointer (not counted in Argc)
+     * is appended, such that Argv[Argc] == NULL.
      */
     Argc = 2 + (LoadIdentifier ? 1 : 0) + Count;
 
     /* Calculate the total size needed for the string buffer of the argument vector */
     Size = 0;
     /* i == 0: Program name */
+    // TODO: Provide one in the future...
     /* i == 1: SystemPartition : from where FreeLdr has been started */
     Size += (strlen("SystemPartition=") + strlen(FrLdrBootPath) + 1) * sizeof(CHAR);
-    /* i == 2: LoadIdentifier  : ASCII string that may be used to associate an identifier with a set of load parameters */
+    /* i == 2: LoadIdentifier  : ASCII string that may be used
+     * to associate an identifier with a set of load parameters */
     if (LoadIdentifier)
     {
         Size += (strlen("LoadIdentifier=") + strlen(LoadIdentifier) + 1) * sizeof(CHAR);
     }
+    /* The section items */
     for (i = 0; i < Count; ++i)
     {
         Size += IniGetSectionSettingNameSize(SectionId, i);  // Counts also the NULL-terminator, that we transform into the '=' sign separator.
@@ -155,15 +161,15 @@ BuildArgvForOsLoader(
     Size += sizeof(ANSI_NULL); // Final NULL-terminator.
 
     /* Allocate memory to hold the argument vector: pointers and string buffer */
-    Argv = FrLdrHeapAlloc(Argc * sizeof(PCHAR) + Size, TAG_STRING);
+    Argv = FrLdrHeapAlloc((Argc + 1) * sizeof(PCHAR) + Size, TAG_STRING);
     if (!Argv)
         return NULL;
 
-    /* Initialize the argument vector: loop through the section and copy the key=value options */
-    SettingName = (PCHAR)((ULONG_PTR)Argv + (Argc * sizeof(PCHAR)));
+    /* Initialize the argument vector: loop through the section and copy the Key=Value options */
+    SettingName = (PCHAR)((ULONG_PTR)Argv + ((Argc + 1) * sizeof(PCHAR)));
     Args = Argv;
     /* i == 0: Program name */
-    *Args++ = NULL;
+    *Args++ = NULL; // TODO: Provide one in the future...
     /* i == 1: SystemPartition */
     {
         strcpy(SettingName, "SystemPartition=");
@@ -181,6 +187,7 @@ BuildArgvForOsLoader(
         *Args++ = SettingName;
         SettingName += (strlen(SettingName) + 1);
     }
+    /* The section items */
     for (i = 0; i < Count; ++i)
     {
         Size = IniGetSectionSettingNameSize(SectionId, i);
@@ -193,6 +200,8 @@ BuildArgvForOsLoader(
         *Args++ = SettingName;
         SettingName += (strlen(SettingName) + 1);
     }
+    /* Terminating NULL pointer */
+    *Args = NULL;
 
 #if DBG
     /* Dump the argument vector for debugging */