Re: [PATCH spice-protocol] Add ioctls structures to qxl_windows.h

[Frediano Ziglio <fziglio@xxxxxxxxxx>]

On Tue, Aug 16, 2016 at 1:27 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:

>
> This patch defines the structures of ioctls that are used between
> win-vdagent and the qxl-wddm-dod driver.
>
> Signed-off-by: Sameeh Jubran <sameeh@xxxxxxxxxx>
> ---
>  spice/qxl_windows.h | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/spice/qxl_windows.h b/spice/qxl_windows.h
> index bc2ceff..c4776d4 100644
> --- a/spice/qxl_windows.h
> +++ b/spice/qxl_windows.h
> @@ -2,20 +2,28 @@
>  #define _H_QXL_WINDOWS
>
>  #include <spice/types.h>
> -
> +#include <spice/qxl_dev.h>
>  #include <spice/start-packed.h>
>
>  enum {
>      QXL_ESCAPE_SET_CUSTOM_DISPLAY = 0x10001,
> -    QXL_ESCAPE_MONITOR_CONFIG
> +    QXL_ESCAPE_MONITOR_CONFIG,
> +    QXL_ESCAPE_VDAGENT_RUNNING

Didn't we decide to remove this?

We can't remove this unless there is a good working alternative. We use those

enums when calling this function https://msdn.microsoft.com/en-us/library/windows/desktop/dd162708%28v=vs.85%29.aspx?f=255&MSPPError=-2147217396

Was referring to QXL_ESCAPE_VDAGENT_RUNNING, you are currently not using it and

also you are not defining a structure for it. I think it's just some lines changes that do not

own to this change.

>  };
>
> -typedef struct SPICE_ATTR_PACKED QXLEscapeSetCustomDisplay {
> +struct SPICE_ATTR_PACKED QXLEscapeSetCustomDisplay {
>      uint32_t xres;
>      uint32_t yres;
>      uint32_t bpp;
> -} QXLEscapeSetCustomDisplay;
> +};

API change here.

why? only removed unneeded typedef. 

Yes, but this break C API, actually this would make this header not C compatible.

This as using QXLEscapeSetCustomDisplay without struct (like in following line)

would cause an error.

>
> +struct SPICE_ATTR_PACKED QXL_ESCAPE {

I would use QXLEscape instead, all structures/enums use this kind of naming.

Would stick to typedefs too.

> +    uint32_t ioctl;
> +    union {
> +        QXLEscapeSetCustomDisplay custom_display;
> +        QXLHead monitor_config;
> +    };
> +};
>  #include <spice/end-packed.h>
>
>  #endif /* _H_QXL_WINDOWS */

Frediano

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel