Re: MR!3 patch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

Frediano,

Attached is the patch file as created by "git format-patch".  In reading the documentation, git format-patch warns of the many problems using all the email clients that I use.  And, frankly, I do not want to figure out how to avoid these pitfalls.

This is becoming increasing difficult to submit such simple patches.  To contribute,
  • I have had to subscribe to Gitlab (I am a Github user).
  • In using the WebIDE, it created a change set involving the whole file because it replaced the Windows EOL sequence with the UNIX EOL sequence.
  • I created Merge Requests but was directed to provide patches.
  • In providing patches, I was instructed to use "git format-patch" versus "git diff".
I understand that you have a development process (I have since read your Developers page).  That understood, I don't intend to contribute beyond the fixing of the bugs that I have discovered and attempted to rectify.  I am Linux developer and do not have a Windows build environment.  (I had a colleague test the compile on Windows 10.)

I have two small patches (MR!1 and MR!3) to address the issue I documented in Issue #4.  If these patches do not work, I am asking that you choose one of the following at this point:
  1. Use the Merge Requests already in your repo;
  2. Use the diff patches as a seed into your development process;
I would also like someone to answer the questions posed in Issue #4 and also address the lack of documentation for the QXL parameters.

Thank you.

- jss


On Tue, Apr 24, 2018 at 8:01 AM, Jon Stumpf <jon.stumpf@xxxxxxxxx> wrote:
Frediano,

I will this evening when I return home and after I read how to use "git format-patch".

- jss


On Tue, Apr 24, 2018 at 7:00 AM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> Here is the patch for Merge Request 3 (MR!3). The WebIDE must have changed
> the end-of-line sequence as I produced this with "git diff
> --ignore-all-space".

> - jss

Jon, can you provide patches in the form of git changes?
Instead of git diff you can use git format-patch.
Alternatively add some comment and email (I assume "Jon Stumpf" <jon.stumpf@xxxxxxxxx>)
you want to see in the git commit.

Frediano



--
- jss




--
- jss

From 6110c1141a57e6c12aa618e1fa7f019fb6027bcf Mon Sep 17 00:00:00 2001
From: jon-stumpf <jon.stumpf@xxxxxxxxx>
Date: Tue, 24 Apr 2018 20:39:10 -0400
Subject: [PATCH] MR!1.patch
To: spice-devel@xxxxxxxxxxxxxxxxxxxxx

This is my first attempt at using "git format-patch".

Jon Stumpf (1):
  Update QxlDod.cpp: This code appears to be a cut-and-paste of the
    MemorySize code directly above it.  The code setting the registry
    key has been updated but an inappropriate comment and the previous
    error message remained.

 qxldod/QxlDod.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.7.4

From 6110c1141a57e6c12aa618e1fa7f019fb6027bcf Mon Sep 17 00:00:00 2001
From: Jon Stumpf <jon.stumpf@xxxxxxxxx>
Date: Tue, 10 Apr 2018 01:28:45 +0000
Subject: [PATCH] Update QxlDod.cpp: This code appears to be a cut-and-paste of
 the MemorySize code directly above it.  The code setting the registry key has
 been updated but an inappropriate comment and the previous error message
 remained.
To: spice-devel@xxxxxxxxxxxxxxxxxxxxx

---
 qxldod/QxlDod.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
index 4f508bd..5fa9c9a 100755
--- a/qxldod/QxlDod.cpp
+++ b/qxldod/QxlDod.cpp
@@ -1982,7 +1982,7 @@ NTSTATUS QxlDod::RegisterHWInfo(_In_ ULONG Id)
 
     UNICODE_STRING ValueQxlDeviceID;
     RtlInitUnicodeString(&ValueQxlDeviceID, L"QxlDeviceID");
-    DWORD DeviceId = Id; // BDD has no access to video memory
+    DWORD DeviceId = Id;
     Status = ZwSetValueKey(DevInstRegKeyHandle,
                            &ValueQxlDeviceID,
                            0,
@@ -1991,7 +1991,7 @@ NTSTATUS QxlDod::RegisterHWInfo(_In_ ULONG Id)
                            sizeof(DeviceId));
     if (!NT_SUCCESS(Status))
     {
-        DbgPrint(TRACE_LEVEL_ERROR, ("ZwSetValueKey for MemorySize failed with Status: 0x%X\n", Status));
+        DbgPrint(TRACE_LEVEL_ERROR, ("ZwSetValueKey for QxlDeviceID failed with Status: 0x%X\n", Status));
         return Status;
     }
 
-- 
2.7.4


From b32f2a3d5a0938b41188c6b183c747f63b16d0da Mon Sep 17 00:00:00 2001
From: jon-stumpf <jon.stumpf@xxxxxxxxx>
Date: Tue, 24 Apr 2018 21:10:06 -0400
Subject: [PATCH 0/3] *** SUBJECT HERE ***
To: spice-devel@xxxxxxxxxxxxxxxxxxxxx

This is my first attempt at using "git format-patch".

Jon Stumpf (1):
  Update RegisterHWInfo() to take a second parameter, MemSize, so that
    "HardwareInformation.MemorySize" can be properly set in the
    Registry.

 qxldod/QxlDod.cpp | 14 +++++++++++---
 qxldod/QxlDod.h   |  2 +-
 2 files changed, 12 insertions(+), 4 deletions(-)

-- 
2.7.4

From a212abdc9e003341c0f9d58ebf1b135ebb3ab8f3 Mon Sep 17 00:00:00 2001
From: Jon Stumpf <jon.stumpf@xxxxxxxxx>
Date: Sat, 14 Apr 2018 20:59:42 +0000
Subject: [PATCH 1/3] Update RegisterHWInfo() to take a second parameter,
 MemSize, so that "HardwareInformation.MemorySize" can be properly set in the
 Registry.
To: spice-devel@xxxxxxxxxxxxxxxxxxxxx

---
 qxldod/QxlDod.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
index 5fa9c9a..3d50a8f 100755
--- a/qxldod/QxlDod.cpp
+++ b/qxldod/QxlDod.cpp
@@ -189,7 +189,7 @@ NTSTATUS QxlDod::StartDevice(_In_  DXGK_START_INFO*   pDxgkStartInfo,
         return Status;
     }
 
-    Status = RegisterHWInfo(m_pHWDevice->GetId());
+    Status = RegisterHWInfo(m_pHWDevice->GetId(), m_DeviceInfo.SystemMemorySize);
     if (!NT_SUCCESS(Status))
     {
         QXL_LOG_ASSERTION1("RegisterHWInfo failed with status 0x%X\n",
@@ -1920,7 +1920,7 @@ NTSTATUS QxlDod::WriteHWInfoStr(_In_ HANDLE DevInstRegKeyHandle, _In_ PCWSTR psz
     return Status;
 }
 
-NTSTATUS QxlDod::RegisterHWInfo(_In_ ULONG Id)
+NTSTATUS QxlDod::RegisterHWInfo(_In_ ULONG Id, _In_ LARGE_INTEGER MemSize)
 {
     PAGED_CODE();
 
@@ -1967,7 +1967,7 @@ NTSTATUS QxlDod::RegisterHWInfo(_In_ ULONG Id)
     // MemorySize is a ULONG, unlike the others which are all strings
     UNICODE_STRING ValueNameMemorySize;
     RtlInitUnicodeString(&ValueNameMemorySize, L"HardwareInformation.MemorySize");
-    DWORD MemorySize = 0; // BDD has no access to video memory
+    DWORD MemorySize = MemSize;
     Status = ZwSetValueKey(DevInstRegKeyHandle,
                            &ValueNameMemorySize,
                            0,
-- 
2.7.4


From 8635f1e27bdd7d1dbdcfa7eb478b66f20f9fd0c5 Mon Sep 17 00:00:00 2001
From: Jon Stumpf <jon.stumpf@xxxxxxxxx>
Date: Wed, 18 Apr 2018 02:44:33 +0000
Subject: [PATCH 2/3] Fixed reference to MemSize as LARGE_INTEGER is a
 structure and not an scalar.
To: spice-devel@xxxxxxxxxxxxxxxxxxxxx

---
 qxldod/QxlDod.cpp | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
index 3d50a8f..e73a4f9 100755
--- a/qxldod/QxlDod.cpp
+++ b/qxldod/QxlDod.cpp
@@ -1965,9 +1965,17 @@ NTSTATUS QxlDod::RegisterHWInfo(_In_ ULONG Id, _In_ LARGE_INTEGER MemSize)
     }
 
     // MemorySize is a ULONG, unlike the others which are all strings
+    //
+    // Microsoft documents the value as counting megabytes, but observation 
+    // suggests it counts bytes. For instance, to represent 512MB, set the 
+    // value to 0x20000000, not to 0x0200.
+    //
+    // A robust implementation would check that the size in bytes, given as 
+    // 64 bits, is not too big for representation as a DWORD!
+
     UNICODE_STRING ValueNameMemorySize;
     RtlInitUnicodeString(&ValueNameMemorySize, L"HardwareInformation.MemorySize");
-    DWORD MemorySize = MemSize;
+    DWORD MemorySize = MemSize.LowPart;
     Status = ZwSetValueKey(DevInstRegKeyHandle,
                            &ValueNameMemorySize,
                            0,
-- 
2.7.4


From b32f2a3d5a0938b41188c6b183c747f63b16d0da Mon Sep 17 00:00:00 2001
From: Jon Stumpf <jon.stumpf@xxxxxxxxx>
Date: Wed, 18 Apr 2018 02:48:56 +0000
Subject: [PATCH 3/3] Fixed declaration of RegisterHWInfo() to match
 implementation in QxlDod.cpp;
To: spice-devel@xxxxxxxxxxxxxxxxxxxxx

---
 qxldod/QxlDod.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
index 695b83a..dc3c9fc 100755
--- a/qxldod/QxlDod.h
+++ b/qxldod/QxlDod.h
@@ -837,7 +837,7 @@ private:
     QXL_NON_PAGED D3DDDI_VIDEO_PRESENT_SOURCE_ID FindSourceForTarget(D3DDDI_VIDEO_PRESENT_TARGET_ID TargetId, BOOLEAN DefaultToZero);
     NTSTATUS IsVidPnSourceModeFieldsValid(CONST D3DKMDT_VIDPN_SOURCE_MODE* pSourceMode) const;
     NTSTATUS IsVidPnPathFieldsValid(CONST D3DKMDT_VIDPN_PRESENT_PATH* pPath) const;
-    NTSTATUS RegisterHWInfo(_In_ ULONG Id);
+    NTSTATUS RegisterHWInfo(_In_ ULONG Id, _In_ LARGE_INTEGER MemSize);
     QXL_NON_PAGED VOID VsyncTimerProc();
     static QXL_NON_PAGED VOID VsyncTimerProcGate(_In_ _KDPC *dpc, _In_ PVOID context, _In_ PVOID arg1, _In_ PVOID arg2);
     QXL_NON_PAGED VOID IndicateVSyncInterrupt();
-- 
2.7.4


_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]