[edk2-devel] [PATCH 12/16] OvmfPkg/EnrollDefaultKeys: describe functions with leading comment blocks

Laszlo Ersek posted 16 patches 6 years, 9 months ago
[edk2-devel] [PATCH 12/16] OvmfPkg/EnrollDefaultKeys: describe functions with leading comment blocks
Posted by Laszlo Ersek 6 years, 9 months ago
The GetExact(), GetSettings(), PrintSettings(), and ShellAppMain()
functions lack leading comment blocks. Supply those.

While at it, make sure that every such comment block is preceded by two
blank lines.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c | 73 ++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
index e4f6a50e008b..07297c631f38 100644
--- a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
+++ b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
@@ -13,16 +13,17 @@
 #include <Library/DebugLib.h>                    // ASSERT()
 #include <Library/MemoryAllocationLib.h>         // FreePool()
 #include <Library/ShellCEntryLib.h>              // ShellAppMain()
 #include <Library/UefiLib.h>                     // AsciiPrint()
 #include <Library/UefiRuntimeServicesTableLib.h> // gRT
 
 #include "EnrollDefaultKeys.h"
 
+
 /**
   Enroll a set of certificates in a global variable, overwriting it.
 
   The variable will be rewritten with NV+BS+RT+AT attributes.
 
   @param[in] VariableName  The name of the variable to overwrite.
 
   @param[in] VendorGuid    The namespace (ie. vendor GUID) of the variable to
@@ -188,16 +189,54 @@ Out:
   if (EFI_ERROR (Status)) {
     AsciiPrint ("error: %a(\"%s\", %g): %r\n", __FUNCTION__, VariableName,
       VendorGuid, Status);
   }
   return Status;
 }
 
 
+/**
+  Read a UEFI variable into a caller-allocated buffer, enforcing an exact size.
+
+  @param[in] VariableName  The name of the variable to read; passed to
+                           gRT->GetVariable().
+
+  @param[in] VendorGuid    The vendor (namespace) GUID of the variable to read;
+                           passed to gRT->GetVariable().
+
+  @param[out] Data         The caller-allocated buffer that is supposed to
+                           receive the variable's contents. On error, the
+                           contents of Data are indeterminate.
+
+  @param[in] DataSize      The size in bytes that the caller requires the UEFI
+                           variable to have. The caller is responsible for
+                           providing room for DataSize bytes in Data.
+
+  @param[in] AllowMissing  If FALSE, the variable is required to exist. If
+                           TRUE, the variable is permitted to be missing.
+
+  @retval EFI_SUCCESS           The UEFI variable exists, has the required size
+                                (DataSize), and has been read into Data.
+
+  @retval EFI_SUCCESS           The UEFI variable doesn't exist, and
+                                AllowMissing is TRUE. DataSize bytes in Data
+                                have been zeroed out.
+
+  @retval EFI_NOT_FOUND         The UEFI variable doesn't exist, and
+                                AllowMissing is FALSE.
+
+  @retval EFI_BUFFER_TOO_SMALL  The UEFI variable exists, but its size is
+                                greater than DataSize.
+
+  @retval EFI_PROTOCOL_ERROR    The UEFI variable exists, but its size is
+                                smaller than DataSize.
+
+  @return                       Error codes propagated from gRT->GetVariable().
+**/
 STATIC
 EFI_STATUS
 GetExact (
   IN CHAR16   *VariableName,
   IN EFI_GUID *VendorGuid,
   OUT VOID    *Data,
   IN UINTN    DataSize,
   IN BOOLEAN  AllowMissing
@@ -223,16 +262,41 @@ GetExact (
     AsciiPrint ("error: GetVariable(\"%s\", %g): expected size 0x%Lx, "
       "got 0x%Lx\n", VariableName, VendorGuid, (UINT64)DataSize, (UINT64)Size);
     return EFI_PROTOCOL_ERROR;
   }
 
   return EFI_SUCCESS;
 }
 
+
+/**
+  Populate a SETTINGS structure from the underlying UEFI variables.
+
+  The following UEFI variables are standard variables:
+  - L"SetupMode"  (EFI_SETUP_MODE_NAME)
+  - L"SecureBoot" (EFI_SECURE_BOOT_MODE_NAME)
+  - L"VendorKeys" (EFI_VENDOR_KEYS_VARIABLE_NAME)
+
+  The following UEFI variables are edk2 extensions:
+  - L"SecureBootEnable" (EFI_SECURE_BOOT_ENABLE_NAME)
+  - L"CustomMode"       (EFI_CUSTOM_MODE_NAME)
+
+  The L"SecureBootEnable" UEFI variable is permitted to be missing, in which
+  case the corresponding field in the SETTINGS object will be zeroed out. The
+  rest of the covered UEFI variables are required to exist; otherwise, the
+  function will fail.
+
+  @param[out] Settings  The SETTINGS object to fill.
+
+  @retval EFI_SUCCESS  Settings has been populated.
+
+  @return              Error codes propagated from the GetExact() function. The
+                       contents of Settings are indeterminate.
+**/
 STATIC
 EFI_STATUS
 GetSettings (
   OUT SETTINGS *Settings
   )
 {
   EFI_STATUS Status;
 
@@ -261,28 +325,37 @@ GetSettings (
     return Status;
   }
 
   Status = GetExact (EFI_VENDOR_KEYS_VARIABLE_NAME, &gEfiGlobalVariableGuid,
              &Settings->VendorKeys, sizeof Settings->VendorKeys, FALSE);
   return Status;
 }
 
+
+/**
+  Print the contents of a SETTINGS structure to the UEFI console.
+
+  @param[in] Settings  The SETTINGS object to print the contents of.
+**/
 STATIC
 VOID
 PrintSettings (
   IN CONST SETTINGS *Settings
   )
 {
   AsciiPrint ("info: SetupMode=%d SecureBoot=%d SecureBootEnable=%d "
     "CustomMode=%d VendorKeys=%d\n", Settings->SetupMode, Settings->SecureBoot,
     Settings->SecureBootEnable, Settings->CustomMode, Settings->VendorKeys);
 }
 
 
+/**
+  Entry point function of this shell application.
+**/
 INTN
 EFIAPI
 ShellAppMain (
   IN UINTN  Argc,
   IN CHAR16 **Argv
   )
 {
   EFI_STATUS Status;
-- 
2.19.1.3.g30247aa5d201



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39687): https://edk2.groups.io/g/devel/message/39687
Mute This Topic: https://groups.io/mt/31359384/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 12/16] OvmfPkg/EnrollDefaultKeys: describe functions with leading comment blocks
Posted by Philippe Mathieu-Daudé 6 years, 9 months ago
On 4/27/19 2:53 AM, Laszlo Ersek wrote:
> The GetExact(), GetSettings(), PrintSettings(), and ShellAppMain()
> functions lack leading comment blocks. Supply those.
> 
> While at it, make sure that every such comment block is preceded by two
> blank lines.
> 
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c | 73 ++++++++++++++++++++
>  1 file changed, 73 insertions(+)
> 
> diff --git a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
> index e4f6a50e008b..07297c631f38 100644
> --- a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
> +++ b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
> @@ -13,16 +13,17 @@
>  #include <Library/DebugLib.h>                    // ASSERT()
>  #include <Library/MemoryAllocationLib.h>         // FreePool()
>  #include <Library/ShellCEntryLib.h>              // ShellAppMain()
>  #include <Library/UefiLib.h>                     // AsciiPrint()
>  #include <Library/UefiRuntimeServicesTableLib.h> // gRT
>  
>  #include "EnrollDefaultKeys.h"
>  
> +
>  /**
>    Enroll a set of certificates in a global variable, overwriting it.
>  
>    The variable will be rewritten with NV+BS+RT+AT attributes.
>  
>    @param[in] VariableName  The name of the variable to overwrite.
>  
>    @param[in] VendorGuid    The namespace (ie. vendor GUID) of the variable to
> @@ -188,16 +189,54 @@ Out:
>    if (EFI_ERROR (Status)) {
>      AsciiPrint ("error: %a(\"%s\", %g): %r\n", __FUNCTION__, VariableName,
>        VendorGuid, Status);
>    }
>    return Status;
>  }
>  
>  
> +/**
> +  Read a UEFI variable into a caller-allocated buffer, enforcing an exact size.
> +
> +  @param[in] VariableName  The name of the variable to read; passed to
> +                           gRT->GetVariable().
> +
> +  @param[in] VendorGuid    The vendor (namespace) GUID of the variable to read;
> +                           passed to gRT->GetVariable().
> +
> +  @param[out] Data         The caller-allocated buffer that is supposed to
> +                           receive the variable's contents. On error, the
> +                           contents of Data are indeterminate.
> +
> +  @param[in] DataSize      The size in bytes that the caller requires the UEFI
> +                           variable to have. The caller is responsible for
> +                           providing room for DataSize bytes in Data.
> +
> +  @param[in] AllowMissing  If FALSE, the variable is required to exist. If
> +                           TRUE, the variable is permitted to be missing.
> +
> +  @retval EFI_SUCCESS           The UEFI variable exists, has the required size
> +                                (DataSize), and has been read into Data.
> +
> +  @retval EFI_SUCCESS           The UEFI variable doesn't exist, and
> +                                AllowMissing is TRUE. DataSize bytes in Data
> +                                have been zeroed out.
> +
> +  @retval EFI_NOT_FOUND         The UEFI variable doesn't exist, and
> +                                AllowMissing is FALSE.
> +
> +  @retval EFI_BUFFER_TOO_SMALL  The UEFI variable exists, but its size is
> +                                greater than DataSize.
> +
> +  @retval EFI_PROTOCOL_ERROR    The UEFI variable exists, but its size is
> +                                smaller than DataSize.
> +
> +  @return                       Error codes propagated from gRT->GetVariable().
> +**/
>  STATIC
>  EFI_STATUS
>  GetExact (
>    IN CHAR16   *VariableName,
>    IN EFI_GUID *VendorGuid,
>    OUT VOID    *Data,
>    IN UINTN    DataSize,
>    IN BOOLEAN  AllowMissing
> @@ -223,16 +262,41 @@ GetExact (
>      AsciiPrint ("error: GetVariable(\"%s\", %g): expected size 0x%Lx, "
>        "got 0x%Lx\n", VariableName, VendorGuid, (UINT64)DataSize, (UINT64)Size);
>      return EFI_PROTOCOL_ERROR;
>    }
>  
>    return EFI_SUCCESS;
>  }
>  
> +
> +/**
> +  Populate a SETTINGS structure from the underlying UEFI variables.
> +
> +  The following UEFI variables are standard variables:
> +  - L"SetupMode"  (EFI_SETUP_MODE_NAME)
> +  - L"SecureBoot" (EFI_SECURE_BOOT_MODE_NAME)
> +  - L"VendorKeys" (EFI_VENDOR_KEYS_VARIABLE_NAME)
> +
> +  The following UEFI variables are edk2 extensions:
> +  - L"SecureBootEnable" (EFI_SECURE_BOOT_ENABLE_NAME)
> +  - L"CustomMode"       (EFI_CUSTOM_MODE_NAME)
> +
> +  The L"SecureBootEnable" UEFI variable is permitted to be missing, in which
> +  case the corresponding field in the SETTINGS object will be zeroed out. The
> +  rest of the covered UEFI variables are required to exist; otherwise, the
> +  function will fail.
> +
> +  @param[out] Settings  The SETTINGS object to fill.
> +
> +  @retval EFI_SUCCESS  Settings has been populated.
> +
> +  @return              Error codes propagated from the GetExact() function. The
> +                       contents of Settings are indeterminate.
> +**/
>  STATIC
>  EFI_STATUS
>  GetSettings (
>    OUT SETTINGS *Settings
>    )
>  {
>    EFI_STATUS Status;
>  
> @@ -261,28 +325,37 @@ GetSettings (
>      return Status;
>    }
>  
>    Status = GetExact (EFI_VENDOR_KEYS_VARIABLE_NAME, &gEfiGlobalVariableGuid,
>               &Settings->VendorKeys, sizeof Settings->VendorKeys, FALSE);
>    return Status;
>  }
>  
> +
> +/**
> +  Print the contents of a SETTINGS structure to the UEFI console.
> +
> +  @param[in] Settings  The SETTINGS object to print the contents of.
> +**/
>  STATIC
>  VOID
>  PrintSettings (
>    IN CONST SETTINGS *Settings
>    )
>  {
>    AsciiPrint ("info: SetupMode=%d SecureBoot=%d SecureBootEnable=%d "
>      "CustomMode=%d VendorKeys=%d\n", Settings->SetupMode, Settings->SecureBoot,
>      Settings->SecureBootEnable, Settings->CustomMode, Settings->VendorKeys);
>  }
>  
>  
> +/**
> +  Entry point function of this shell application.
> +**/
>  INTN
>  EFIAPI
>  ShellAppMain (
>    IN UINTN  Argc,
>    IN CHAR16 **Argv
>    )
>  {
>    EFI_STATUS Status;
> 

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39824): https://edk2.groups.io/g/devel/message/39824
Mute This Topic: https://groups.io/mt/31359384/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-