On 10/20/12 00:38, Jordan Justen wrote:
> EFI_STATUS
> +TryRunningQemuKernel (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + UINTN Index;
> + UINTN FvHandleCount;
> + EFI_HANDLE *FvHandleBuffer;
> + EFI_DEVICE_PATH_PROTOCOL *DevicePath;
> + EFI_HANDLE ImageHandle;
> + MEDIA_FW_VOL_FILEPATH_DEVICE_PATH ShellNode;
> +
> + //
> + // Check if we have on flash shell
> + //
> + gBS->LocateHandleBuffer (
> + ByProtocol,
> + &gEfiFirmwareVolume2ProtocolGuid,
> + NULL,
> + &FvHandleCount,
> + &FvHandleBuffer
> + );
> + for (Index = 0; Index < FvHandleCount; Index++) {
> + gBS->HandleProtocol (
> + FvHandleBuffer[Index],
> + &gEfiDevicePathProtocolGuid,
> + (VOID **) &DevicePath
> + );
We're probably sure that any firmware volume comes with a device path,
but I guess an ASSERT() wouldn't hurt.
> +
> + //
> + // Build device path for QEMU Kernel executable
> + //
> + EfiInitializeFwVolDevicepathNode (&ShellNode, &gOvmfQemuKernelFile);
> + DevicePath = AppendDevicePathNode (DevicePath, (EFI_DEVICE_PATH_PROTOCOL
> *) &ShellNode);
AppendDevicePathNode() can run out of memory.
> + Status = gBS->LoadImage (
> + TRUE,
> + gImageHandle,
> + DevicePath,
> + NULL,
> + 0,
> + &ImageHandle
> + );
> + if (!EFI_ERROR (Status)) {
> + Status = gBS->StartImage (ImageHandle, NULL, NULL);
Optimally this doesn't return. If it does: shouldn't we unload the image?
... It would probably be useless; the image has been started and has no
Unload() entry point; we're bound to leak it. (I think that's the common
case, -kernel should be passed to qemu infrequently.)
> + break;
> + }
We also leak the appended device path. (We leak one appended device path
for each firmware volume.)
> + }
We leak FvHandleBuffer. (At least one firmware volume exists.)
> +
> + return EFI_SUCCESS;
> +}
I think we should return Status here (which should be initialized to
EFI_NOT_FOUND at the top, perhaps). Granted, the return value is ignored.
Out of curiosity: why did you implement this functionality as an UEFI
application? I believe a regular function call would do fine in place of
LoadImage / StartImage.
Thanks,
Laszlo
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
