>
> This patch fixes code flow in StartDriver function.
>
> Based on a patch by Sandy Stutsman <sstutsma@xxxxxxxxxx>
>
> Signed-off-by: Sameeh Jubran <sameeh@xxxxxxxxxx>
The extra indentation is not a great enhancement.
The do {} while is quite useless.
The style changes without a clear coding style document are not
justified.
But the big problem is that the initial rationale (setting DriverStarted to FALSE)
were supposed to be moved in a different patch which seems missing (maybe I'm wrong).
> ---
> qxldod/QxlDod.cpp | 107
> ++++++++++++++++++++++++++----------------------------
> 1 file changed, 52 insertions(+), 55 deletions(-)
>
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index dacfca3..dc83f26 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -113,72 +113,71 @@ NTSTATUS QxlDod::StartDevice(_In_ DXGK_START_INFO*
> pDxgkStartInfo,
> {
> PHYSICAL_ADDRESS PhysicAddress;
> PAGED_CODE();
> + NTSTATUS Status;
> QXL_ASSERT(pDxgkStartInfo != NULL);
> QXL_ASSERT(pDxgkInterface != NULL);
> QXL_ASSERT(pNumberOfViews != NULL);
> QXL_ASSERT(pNumberOfChildren != NULL);
> -//CHECK ME!!!!!!!!!!!!!
> + //CHECK ME!!!!!!!!!!!!!
> RtlCopyMemory(&m_DxgkInterface, pDxgkInterface,
> sizeof(m_DxgkInterface));
> RtlZeroMemory(m_CurrentModes, sizeof(m_CurrentModes));
> -//CHECK ME!!!!!!!!!!!!!
> + //CHECK ME!!!!!!!!!!!!!
> m_CurrentModes[0].DispInfo.TargetId = D3DDDI_ID_UNINITIALIZED;
these indentation fix are fine, I would go to a more standard FIXME or TODO.
> - // Get device information from OS.
> - NTSTATUS Status =
> m_DxgkInterface.DxgkCbGetDeviceInformation(m_DxgkInterface.DeviceHandle,
> &m_DeviceInfo);
> - if (!NT_SUCCESS(Status))
> - {
> - QXL_LOG_ASSERTION1("DxgkCbGetDeviceInformation failed with status
> 0x%X\n",
> - Status);
> - return Status;
> - }
> + do {
>
> - Status = CheckHardware();
> - if (NT_SUCCESS(Status))
> - {
> - m_pHWDevice = new(NonPagedPoolNx) QxlDevice(this);
> - }
> - else
> - {
> - m_pHWDevice = new(NonPagedPoolNx) VgaDevice(this);
> - }
> + // Get device information from OS.
> + Status =
> m_DxgkInterface.DxgkCbGetDeviceInformation(m_DxgkInterface.DeviceHandle,
> &m_DeviceInfo);
> + if (!NT_SUCCESS(Status)) {
> + QXL_LOG_ASSERTION1("DxgkCbGetDeviceInformation failed with
> status 0x%X\n",
> + Status);
> + break;
> + }
>
> - if (!m_pHWDevice)
> - {
> - Status = STATUS_NO_MEMORY;
> - DbgPrint(TRACE_LEVEL_ERROR, ("HWInit failed to allocate memory\n"));
> - return Status;
> - }
> + Status = CheckHardware();
> + if (NT_SUCCESS(Status)) {
> + m_pHWDevice = new(NonPagedPoolNx) QxlDevice(this);
> + }
> + else {
> + m_pHWDevice = new(NonPagedPoolNx) VgaDevice(this);
> + }
>
> - Status = m_pHWDevice->HWInit(m_DeviceInfo.TranslatedResourceList,
> &m_CurrentModes[0].DispInfo);
> - if (!NT_SUCCESS(Status))
> - {
> - DbgPrint(TRACE_LEVEL_ERROR, ("HWInit failed with status 0x%X\n",
> Status));
> - return Status;
> - }
> + if (!m_pHWDevice) {
> + Status = STATUS_NO_MEMORY;
> + DbgPrint(TRACE_LEVEL_ERROR, ("HWInit failed to allocate
> memory\n"));
> + break;
> + }
>
> - Status = RegisterHWInfo(m_pHWDevice->GetId());
> - if (!NT_SUCCESS(Status))
> - {
> - QXL_LOG_ASSERTION1("RegisterHWInfo failed with status 0x%X\n",
> - Status);
> - return Status;
> - }
> + Status = m_pHWDevice->HWInit(m_DeviceInfo.TranslatedResourceList,
> &m_CurrentModes[0].DispInfo);
> + if (!NT_SUCCESS(Status)) {
> + DbgPrint(TRACE_LEVEL_ERROR, ("HWInit failed with status 0x%X\n",
> Status));
> + break;
> + }
>
> - PhysicAddress.QuadPart =
> m_CurrentModes[0].DispInfo.PhysicAddress.QuadPart;
> - if (m_pHWDevice->GetId() == 0)
> - {
> - Status =
> m_DxgkInterface.DxgkCbAcquirePostDisplayOwnership(m_DxgkInterface.DeviceHandle,
> &(m_CurrentModes[0].DispInfo));
> - }
> + Status = RegisterHWInfo(m_pHWDevice->GetId());
> + if (!NT_SUCCESS(Status)) {
> + QXL_LOG_ASSERTION1("RegisterHWInfo failed with status 0x%X\n",
> + Status);
> + break;
> + }
>
> - if (!NT_SUCCESS(Status) )
> - {
> - DbgPrint(TRACE_LEVEL_ERROR, ("DxgkCbAcquirePostDisplayOwnership
> failed with status 0x%X Width = %d\n",
> - Status, m_CurrentModes[0].DispInfo.Width));
> - return STATUS_UNSUCCESSFUL;
> + PhysicAddress.QuadPart =
> m_CurrentModes[0].DispInfo.PhysicAddress.QuadPart;
> + if (m_pHWDevice->GetId() == 0) {
> + Status =
> m_DxgkInterface.DxgkCbAcquirePostDisplayOwnership(m_DxgkInterface.DeviceHandle,
> &(m_CurrentModes[0].DispInfo));
> + }
> +
> + if (!NT_SUCCESS(Status)) {
> + DbgPrint(TRACE_LEVEL_ERROR, ("DxgkCbAcquirePostDisplayOwnership
> failed with status 0x%X Width = %d\n",
> + Status, m_CurrentModes[0].DispInfo.Width));
> + Status = STATUS_UNSUCCESSFUL;
> + break;
> + }
> + } while (0);
> + if (!NT_SUCCESS(Status)) {
> + return Status;
> }
>
> - if (m_CurrentModes[0].DispInfo.Width == 0)
> - {
> - m_CurrentModes[0].DispInfo.Width = MIN_WIDTH_SIZE;
> + if (m_CurrentModes[0].DispInfo.Width == 0) {
> + m_CurrentModes[0].DispInfo.Width = MIN_WIDTH_SIZE;
> m_CurrentModes[0].DispInfo.Height = MIN_HEIGHT_SIZE;
> m_CurrentModes[0].DispInfo.Pitch =
> BPPFromPixelFormat(D3DDDIFMT_R8G8B8) / 8;
> m_CurrentModes[0].DispInfo.ColorFormat = D3DDDIFMT_R8G8B8;
All these are just style.
> @@ -186,11 +185,9 @@ NTSTATUS QxlDod::StartDevice(_In_ DXGK_START_INFO*
> pDxgkStartInfo,
> if (PhysicAddress.QuadPart != 0L) {
> m_CurrentModes[0].DispInfo.PhysicAddress.QuadPart =
> PhysicAddress.QuadPart;
> }
> -
> }
> -
> - *pNumberOfViews = MAX_VIEWS;
> - *pNumberOfChildren = MAX_CHILDREN;
> + *pNumberOfViews = MAX_VIEWS;
> + *pNumberOfChildren = MAX_CHILDREN;
These indentation fix are fine.
> m_Flags.DriverStarted = TRUE;
> DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));
> return STATUS_SUCCESS;
Can Windows stop and start a device again?
In this case you would have a leak.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel