[Spice-devel] [PATCH qxl-wddm-dod v5 2/7] Code Analysis fix

[Frediano Ziglio]

> 
> The PAGED_CODE macro ensures that the calling thread is running at an
> IRQL that is low enough to permit paging. A call to this macro should
> be made at the beginning of every driver routine that either contains
> pageable code or accesses pageable code.
> 
> Based on a patch by Sandy Stutsman <sstutsma at redhat.com>
> 
> Signed-off-by: Sameeh Jubran <sameeh at daynix.com>
> ---
>  qxldod/QxlDod.cpp | 224
>  ++++++++++++++++++++++++++++++++++++++++++++++++------
>  qxldod/QxlDod.h   | 149 ++++++++++++++++++++++++++++++++++--
>  2 files changed, 344 insertions(+), 29 deletions(-)
> 
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index 798b2f0..136daf7 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -2,8 +2,6 @@
>  #include "qxldod.h"
>  #include "qxl_windows.h"
>  
> -#pragma code_seg(push)
> -#pragma code_seg()
>  
>  #define WIN_QXL_INT_MASK ((QXL_INTERRUPT_DISPLAY) | \
>                            (QXL_INTERRUPT_CURSOR) | \
> @@ -47,7 +45,6 @@ BYTE PixelMask[BITS_PER_BYTE]  = {0x80, 0x40, 0x20, 0x10,
> 0x08, 0x04, 0x02, 0x01
>                                         ((ULONG)LOWER_5_BITS((Pixel)) <<
>                                         SHIFT_LOWER_5_IN_565_BACK))
>  
>  
> -#pragma code_seg(pop)
>  
>  struct QXLEscape {
>      uint32_t ioctl;
> @@ -57,10 +54,12 @@ struct QXLEscape {
>      };
>  };
>  
> +__declspec(code_seg("PAGE"))
>  QxlDod::QxlDod(_In_ DEVICE_OBJECT* pPhysicalDeviceObject) :
>  m_pPhysicalDevice(pPhysicalDeviceObject),
>                                                              m_MonitorPowerState(PowerDeviceD0),
>                                                              m_AdapterPowerState(PowerDeviceD0)
>  {
> +    PAGED_CODE();
>      DbgPrint(TRACE_LEVEL_INFORMATION, ("---> %s\n", __FUNCTION__));
>      *((UINT*)&m_Flags) = 0;
>      RtlZeroMemory(&m_DxgkInterface, sizeof(m_DxgkInterface));
> @@ -71,7 +70,7 @@ QxlDod::QxlDod(_In_ DEVICE_OBJECT* pPhysicalDeviceObject) :
> m_pPhysicalDevice(pP
>      DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));
>  }
>  
> -
> +__declspec(code_seg("PAGE"))
>  QxlDod::~QxlDod(void)
>  {
>      PAGED_CODE();
> @@ -80,6 +79,7 @@ QxlDod::~QxlDod(void)
>      m_pHWDevice = NULL;
>  }
>  
> +__declspec(code_seg("PAGE"))
>  NTSTATUS QxlDod::CheckHardware()
>  {
>      PAGED_CODE();

...

The patch looks ok but all the __declspec(code_seg("PAGE")) are quite ugly.
I attempted to write a similar fix on the other way (specifying the non paged part)
but I agree that specifying the paged code is a much safer solution as missing
a line would make the code non paged so no problems.
However I would use a macro for all that declarations, something like

#define QXL_PAGED __declspec(code_seg("PAGE"))

(or even PAGED). The replace can be made easily with a tool.

Still the patch is quite huge, I'll wait some comment.

Frediano