Re: [PATCH] MdePkg: correct and clarify documentation of VA_LIST in Base.h

[Laszlo Ersek <lersek@redhat.com>]

On 12/12/17 11:24, Zenith432 wrote:
> Below is an amended patch.
> 
> I changed the wording in the notes a little to exclude a non-variadic
> function using VA_START, and to remind that VA_COPY should also be
> paired with VA_END.
> 
> Also, if GCC gets its VA_COPY repaired, documentation may change to
> allow to using VA_COPY
> inside non-EFIAPI as well.
> ---
> 
> This is to resolve bug 457.
> https://bugzilla.tianocore.org/show_bug.cgi?id=457
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zenith432 <zenith432 at users.sourceforge.net>
> ---
>  MdePkg/Include/Base.h | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
> index 02140a5a..4fd5161f 100644
> --- a/MdePkg/Include/Base.h
> +++ b/MdePkg/Include/Base.h
> @@ -552,21 +552,24 @@ struct _LIST_ENTRY {
>  #define  BASE_8EB    0x8000000000000000ULL
> 
>  //
> -//  Support for variable length argument lists using the ANSI standard.
> +//  Support for variable argument lists in freestanding edk2 modules.
>  //
> -//  Since we are using the ANSI standard we used the standard naming and
> -//  did not follow the coding convention
> +//  For modules that use the ISO C library interfaces for variable
> +//  argument lists, refer to "StdLib/Include/stdarg.h".
>  //
>  //  VA_LIST  - typedef for argument list.
>  //  VA_START (VA_LIST Marker, argument before the ...) - Init Marker
> for use.
>  //  VA_END (VA_LIST Marker) - Clear Marker
> -//  VA_ARG (VA_LIST Marker, var arg size) - Use Marker to get an
> argument from
> -//    the ... list. You must know the size and pass it in this macro.
> +//  VA_ARG (VA_LIST Marker, var arg type) - Use Marker to get an
> argument from
> +//    the ... list. You must know the type and pass it in this macro. 
> Type
> +//    must be compatible with the type of the actual next argument (as
> promoted
> +//    according to the default argument promotions.)
>  //  VA_COPY (VA_LIST Dest, VA_LIST Start) - Initialize Dest as a copy
> of Start.
>  //
> -//  example:
> +//  Example:
>  //
>  //  UINTN
> +//  EFIAPI
>  //  ExampleVarArg (
>  //    IN UINTN  NumberOfArgs,
>  //    ...
> @@ -582,15 +585,21 @@ struct _LIST_ENTRY {
>  //    VA_START (Marker, NumberOfArgs);
>  //    for (Index = 0, Result = 0; Index < NumberOfArgs; Index++) {
>  //      //
> -//      // The ... list is a series of UINTN values, so average them up.
> +//      // The ... list is a series of UINTN values, so sum them up.
>  //      //
>  //      Result += VA_ARG (Marker, UINTN);
>  //    }
>  //
>  //    VA_END (Marker);
> -//    return Result
> +//    return Result;
>  //  }
>  //
> +//  Notes:
> +//  - Functions that call VA_START() / VA_END() must have a variable
> +//    argument list and must be declared EFIAPI.
> +//  - Functions that call VA_COPY() / VA_END() must be declared EFIAPI.
> +//  - Functions that only use VA_LIST and VA_ARG() need not be EFIAPI.
> +//
> 
>  /**
>    Return the size of argument that has been aligned to sizeof (UINTN).

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

(Liming, I can help with applying this patch, if you want that.)

Thanks!
Laszlo