Re: [PATCH v2 27/29] ArmVirtualizationPkg: add XenIoMmioLib

[Laszlo Ersek <lersek@redhat.com>]

On 02/03/15 12:45, Laszlo Ersek wrote:
> comments below
> 
> On 01/26/15 20:03, Ard Biesheuvel wrote:

>> +EFI_STATUS
>> +XenIoMmioInstall (
>> +  IN  EFI_HANDLE  *Handle,
>> +  IN  UINT64      GrantTableAddress
>> +  )
>> +{
>> +  EFI_STATUS                     Status;
>> +  XENIO_PROTOCOL                 *XenIo;
>> +  XENBUS_ROOT_DEVICE_PATH        *XenBusDevicePath;
>> +
>> +  ASSERT (Handle != NULL);
> 
> (4) This is wrong. (I'm not sure how you are using the library in the
> following patches, but this in itself does look wrong.)

Heh, I tricked myself. This ASSERT() is valid. I missed you weren't
saying *Handle.

However, I believe the following remains valid from my point (4):

> (Note that the uninstall functions can't null the Handle parameter in
> that case -- which means that you should save a copy of the incoming
> handle, and restore it at the end when something fails. If it was
> non-NULL initially, nothing will change, but if it was NULL initially,
> you have to reset it to NULL.)

Thanks
Laszlo