Re: [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib

[Jordan Justen <jordan.l.justen@intel.com>]

On 2019-03-14 22:17:33, Zhichao Gao wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> 
> Add a new api DebugVPrint prototype definition in the
> DebugLib header file. This api would expose a print
> routine with VaList parameter.

These lines seem to be fairly short, with the longest be 54 chars. I
guess not a problem, but by the recommendation says they could be up
to 75 in length.

https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format

> 
> diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h
> index e6a7a357b2..51d89bbd52 100644
> --- a/MdePkg/Include/Library/DebugLib.h
> +++ b/MdePkg/Include/Library/DebugLib.h
> @@ -8,7 +8,7 @@
>    of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is
>    defined, then debug and assert related macros wrapped by it are the NULL implementations.
>  
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials are licensed and made available under
>  the terms and conditions of the BSD License that accompanies this distribution.
>  The full text of the license may be found at
> @@ -101,6 +101,29 @@ DebugPrint (
>    );
>  
> +/**
> +  Prints a debug message to the debug output device if the specified error level is enabled.

According to the style guide:

  "Preferably, limit line lengths to 80 columns or less."

https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C

https://github.com/tianocore-docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf

But, this line uses 92 columns.

I think there are similar cases in other patches.

> +
> +  If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib function
> +  GetDebugPrintErrorLevel (), then print the message specified by Format and the
> +  associated variable argument list to the debug output device.
> +
> +  If Format is NULL, then ASSERT().
> +
> +  @param  ErrorLevel    The error level of the debug message.
> +  @param  Format        Format string for the debug message to print.
> +  @param  VaListMarker  VA_LIST marker for the variable argument list.
> +
> +**/
> +VOID
> +EFIAPI
> +DebugVPrint (
> +  IN  UINTN        ErrorLevel,
> +  IN  CONST CHAR8  *Format,
> +  IN  VA_LIST       VaListMarker
> +  );
> +
> +
>  /**
>    Prints an assert message containing a filename, line number, and description.
>    This may be followed by a breakpoint or a dead loop.
> @@ -221,6 +244,7 @@ DebugClearMemoryEnabled (
>    VOID
>    );
>  
> +

What's going on here? It seems like maybe extra lines are added that
have nothing to do with this patch.

-Jordan

>  /**
>    Returns TRUE if any one of the bit is set both in ErrorLevel and PcdFixedDebugPrintErrorLevel.
>  
> @@ -236,6 +260,7 @@ DebugPrintLevelEnabled (
>    IN  CONST UINTN        ErrorLevel
>    );
>  
> +
>  /**
>    Internal worker macro that calls DebugAssert().
>  
> @@ -273,6 +298,7 @@ DebugPrintLevelEnabled (
>  #define _DEBUG(Expression)   DebugPrint Expression
>  #endif
>  
> +
>  /**
>    Macro that calls DebugAssert() if an expression evaluates to FALSE.
>  
> @@ -299,6 +325,7 @@ DebugPrintLevelEnabled (
>    #define ASSERT(Expression)
>  #endif
>  
> +
>  /**
>    Macro that calls DebugPrint().
>  
> @@ -322,6 +349,7 @@ DebugPrintLevelEnabled (
>    #define DEBUG(Expression)
>  #endif
>  
> +
>  /**
>    Macro that calls DebugAssert() if an EFI_STATUS evaluates to an error code.
>  
> @@ -348,6 +376,7 @@ DebugPrintLevelEnabled (
>    #define ASSERT_EFI_ERROR(StatusParameter)
>  #endif
>  
> +
>  /**
>    Macro that calls DebugAssert() if a RETURN_STATUS evaluates to an error code.
>  
> @@ -375,6 +404,7 @@ DebugPrintLevelEnabled (
>    #define ASSERT_RETURN_ERROR(StatusParameter)
>  #endif
>  
> +
>  /**
>    Macro that calls DebugAssert() if a protocol is already installed in the
>    handle database.
> @@ -418,6 +448,7 @@ DebugPrintLevelEnabled (
>    #define ASSERT_PROTOCOL_ALREADY_INSTALLED(Handle, Guid)
>  #endif
>  
> +
>  /**
>    Macro that marks the beginning of debug source code.
>  
> -- 
> 2.16.2.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel