Re: Possible UEFI Shell DrvDiag Command Error

["Carsey, Jaben" <jaben.carsey@intel.com>]

I would say the shell should not free the GUID.

> -----Original Message-----
> From: Jim.Dailey@dell.com [mailto:Jim.Dailey@dell.com]
> Sent: Monday, April 24, 2017 5:46 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Carsey, Jaben <jaben.carsey@intel.com>
> Subject: [edk2] Possible UEFI Shell DrvDiag Command Error
> Importance: High
> 
> Although this question is about the shell's drvdiag command, the answer
> is related to the interpretation of the UEFI specification with respect
> to the diagnostics protocol's RunDiagnostics() function.
> 
> Ultimately, the question is, is the drvdiag command in error in trying
> to free the ErrorType pointer, or is a driver vendor in error when they
> return an ErrorType parameter that does not point to memory that was
> allocated using gBS->AllocatePool?
> 
> The diagnostics protocol's RunDiagnostics() function is partially
> described in the UEFI spec like this:
> 
>   typedef
>   EFI_STATUS
>   (EFIAPI *EFI_DRIVER_DIAGNOSTICS2_RUN_DIAGNOSTICS) (
>     IN EFI_DRIVER_DIAGNOSTICS2_PROTOCOL *This,
>     ...
>     OUT EFI_GUID                        **ErrorType,
>     OUT UINTN                           *BufferSize,
>     OUT CHAR16                          **Buffer
>     );
> 
>   ...
> 
>   ErrorType       A GUID that defines the format of the data returned in
>                   Buffer.
>   BufferSize      The size, in bytes, of the data returned in Buffer.
>   Buffer          A buffer that contains a Null-terminated string plus
>                   some additional data whose format is defined by
>                   ErrorType. Buffer is allocated by this function with
>                   EFI_BOOT_SERVICES.AllocatePool(), and it is the
>                   caller's responsibility to free it with a call to
>                   EFI_BOOT_SERVICES.FreePool().
> 
> After a call to RunDiagnostics, the drvdiag command attempts to free the
> ErrorType pointer if it is non-NULL, yet nothing in the UEFI spec says
> that this is proper.
> 
> 1234567890123456789012345678901234567890123456789012345678901234567
> 89012
> The UEFI spec goes to great length to point out that *Buffer* must be
> allocated by the callee using gBS->AllocatePool and that the caller must
> use gBS->FreePool to release the memory once it is no longer needed.
> However, there is no such language with respect to ErrorType.
> 
> So, repeating the question, is the drvdiag command in error in trying
> to free the ErrorType pointer, or is a driver vendor in error when they
> return an ErrorType parameter that does not point to memory that was
> allocated using gBS->AllocatePool?
> 
> Regards,
> Jim