<html><body><div style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><div>About typedef of struct is true, all structures, even declared in C++ have typedefs.</div><div>About all upper case is false, beside the definitions added by these patches but are C definitions, not C++.</div><div>About space there is a space after the structure name before bracket ("{").<br></div><div>About empty lines mostly all structure declaration have a comment or an empty line before.<br></div><div><br></div><div>Frediano<br></div><div><br></div><div><br></div><hr id="zwchr"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><b>From: </b>"Sameeh Jubran" <sameeh@daynix.com><br><b>To: </b>"Frediano Ziglio" <fziglio@redhat.com><br><b>Cc: </b>"Dmitry Fleytman" <dmitry@daynix.com>, spice-devel@lists.freedesktop.org<br><b>Sent: </b>Wednesday, July 20, 2016 6:11:27 PM<br><b>Subject: </b>Re: [Spice-devel] [RFC PATCH vdagent 14/16] Adding ioctl operation to update Vdagent state<br><div><br></div><div dir="ltr">This style was used in order to match the current style of VDAgent.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jul 18, 2016 at 10:54 AM, Frediano Ziglio <span dir="ltr"><<a href="mailto:fziglio@redhat.com" target="_blank">fziglio@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">><br> > From: Sameeh Jubran <<a href="mailto:sameeh@daynix.com" target="_blank">sameeh@daynix.com</a>><br> ><br> > This patch adds new ioctl operation to Vdagent in order to update<br> > the driver on Vdagent state. This allows the driver to know<br> > when Vdagent is running and when it is off.<br> ><br> > Signed-off-by: Sameeh Jubran <<a href="mailto:sameeh@daynix.com" target="_blank">sameeh@daynix.com</a>><br> > Signed-off-by: Dmitry Fleytman <<a href="mailto:dmitry@daynix.com" target="_blank">dmitry@daynix.com</a>><br> > ---<br> > vdagent/desktop_layout.cpp | 21 +++++++++++++++++++++<br> > vdagent/desktop_layout.h | 1 +<br> > vdagent/display_configuration.cpp | 15 +++++++++++++++<br> > vdagent/display_configuration.h | 2 ++<br> > 4 files changed, 39 insertions(+)<br> ><br> > diff --git a/vdagent/desktop_layout.cpp b/vdagent/desktop_layout.cpp<br> > index 7aff7e7..a8d9e2d 100644<br> > --- a/vdagent/desktop_layout.cpp<br> > +++ b/vdagent/desktop_layout.cpp<br> > @@ -44,6 +44,7 @@ DesktopLayout::DesktopLayout()<br> ><br> > DesktopLayout::~DesktopLayout()<br> > {<br> > + set_vdagent_running_for_displays(false);<br> > clean_displays();<br> > if (_display_config){<br> > delete _display_config;<br> > @@ -122,6 +123,8 @@ void DesktopLayout::set_displays()<br> > DWORD display_id = 0;<br> > int dev_sets = 0;<br> ><br> > + set_vdagent_running_for_displays(true);<br> > +<br> > lock();<br> > if (!consistent_displays()) {<br> > unlock();<br> > @@ -276,6 +279,22 @@ bool DesktopLayout::get_qxl_device_id(WCHAR* device_key,<br> > DWORD* device_id)<br> > return key_found;<br> > }<br> ><br> > +void DesktopLayout::set_vdagent_running_for_displays(bool running)<br> > +{<br> > + DISPLAY_DEVICE dev_info;<br> > + DWORD dev_id = 0;<br> > + lock();<br> > + ZeroMemory(&dev_info, sizeof(dev_info));<br> > + dev_info.cb = sizeof(dev_info);<br> > + while (EnumDisplayDevices(NULL, dev_id, &dev_info, 0)) {<br> > + dev_id++;<br> > + if (!(dev_info.StateFlags & DISPLAY_DEVICE_MIRRORING_DRIVER) &&<br> > wcsstr(dev_info.DeviceString, L"QXL")) {<br> > + _display_config->set_vdagent_running(dev_info.DeviceName,<br> > running);<br> > + }<br> > + }<br> > + unlock();<br> > +}<br> > +<br> > bool DesktopLayout::init_dev_mode(LPCTSTR dev_name, DEVMODE* dev_mode,<br> > DisplayMode* mode)<br> > {<br> > ZeroMemory(dev_mode, sizeof(DEVMODE));<br> > @@ -299,6 +318,8 @@ bool DesktopLayout::init_dev_mode(LPCTSTR dev_name,<br> > DEVMODE* dev_mode, DisplayMo<br> > // update current DisplayMode (so mouse scaling works properly)<br> > mode->_width = dev_mode->dmPelsWidth;<br> > mode->_height = dev_mode->dmPelsHeight;<br> > +<br> > + set_vdagent_running_for_displays(true);<br> > return true;<br> ><br> > }<br> > diff --git a/vdagent/desktop_layout.h b/vdagent/desktop_layout.h<br> > index fd6af76..da5a40b 100644<br> > --- a/vdagent/desktop_layout.h<br> > +++ b/vdagent/desktop_layout.h<br> > @@ -83,6 +83,7 @@ private:<br> > static bool consistent_displays();<br> > static bool is_attached(LPCTSTR dev_name);<br> > static bool get_qxl_device_id(WCHAR* device_key, DWORD* device_id);<br> > + void set_vdagent_running_for_displays(bool enable_pointer);<br> > private:<br> > mutex_t _mutex;<br> > Displays _displays;<br> > diff --git a/vdagent/display_configuration.cpp<br> > b/vdagent/display_configuration.cpp<br> > index f20605a..95fd4c3 100644<br> > --- a/vdagent/display_configuration.cpp<br> > +++ b/vdagent/display_configuration.cpp<br> > @@ -67,6 +67,15 @@ typedef struct WDDM_MONITOR_CONFIG_ESCAPE {<br> > int _ioctl;<br> > QXLHead _head;<br> > } WDDM_MONITOR_CONFIG_ESCAPE;<br> > +typedef struct WDDM_VDAGENT_RUNNING_ESCAPE{<br> > + WDDM_VDAGENT_RUNNING_ESCAPE(bool running)<br> > + {<br> > + _ioctl = QXL_ESCAPE_VDAGENT_RUNNING;<br> > + _vdagent_state.running = running;<br> > + }<br> > + int _ioctl;<br> > + QXLEscapeVDAgentRunning _vdagent_state;<br> > +} WDDM_VDAGENT_RUNNING_ESCAPE;<br> <br> </div></div>This style is really confusing, it's a mix of C and C++,<br> there is no reason for the typedef.<br> I think also there is some missing empty line and space.<br> Also the choice of all capital is confusing (maybe just me)<br> <div class="HOEnZb"><div class="h5"><br> > #endif<br> > DisplayConfig* DisplayConfig::create_config()<br> > {<br> > @@ -529,6 +538,12 @@ bool WDDMInterface::escape(LPCTSTR device_name, void*<br> > data, UINT size_data)<br> > return NT_SUCCESS(status);<br> > }<br> ><br> > +bool WDDMInterface::set_vdagent_running(LPCTSTR device_name, bool running)<br> > +{<br> > + WDDM_VDAGENT_RUNNING_ESCAPE wddm_escape(running);<br> > + return escape(device_name, &wddm_escape, sizeof(wddm_escape));<br> > +}<br> > +<br> > CCD::CCD()<br> > :_NumPathElements(0)<br> > ,_NumModeElements(0)<br> > diff --git a/vdagent/display_configuration.h<br> > b/vdagent/display_configuration.h<br> > index eeba1c2..06f592a 100644<br> > --- a/vdagent/display_configuration.h<br> > +++ b/vdagent/display_configuration.h<br> > @@ -132,6 +132,7 @@ public:<br> > DRIVER_TYPE type() { return _driver_type; };<br> > void set_monitors_config(bool flag) { _send_monitors_config = flag; }<br> > virtual void update_config_path() {};<br> > + virtual bool set_vdagent_running(LPCTSTR device_name, bool running) {<br> > return false; };<br> ><br> > protected:<br> > DRIVER_TYPE _driver_type;<br> > @@ -176,6 +177,7 @@ private:<br> ><br> > void close_adapter(D3DKMT_HANDLE handle);<br> > bool escape(LPCTSTR device_name, void* data, UINT sizeData);<br> > + bool set_vdagent_running(LPCTSTR device_name, bool running);<br> ><br> > private:<br> > //GDI Function pointers<br> <br> </div></div><span class="HOEnZb"><span style="color: #888888;" data-mce-style="color: #888888;" color="#888888">Frediano<br> </span></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><span style="color: #0b5394; font-family: times new roman,serif; font-size: large;" data-mce-style="color: #0b5394; font-family: times new roman,serif; font-size: large;" color="#0b5394" face="times new roman, serif" size="4">Respectfully,<br></span><div style="font-size:12.8px;color:rgb(136,136,136)"><span style="color: #0b5394; font-family: times new roman,serif; font-size: large;" data-mce-style="color: #0b5394; font-family: times new roman,serif; font-size: large;" color="#0b5394" face="times new roman, serif" size="4"><b><i>Sameeh Jubran</i></b></span></div><div style="font-size:12.8px;color:rgb(136,136,136)"><span style="color: #073763; font-family: times new roman,serif; font-size: large;" data-mce-style="color: #073763; font-family: times new roman,serif; font-size: large;" color="#073763" face="times new roman, serif" size="4"><i>Mobile: +972 054-2509642</i></span></div><div style="font-size:12.8px;color:rgb(136,136,136)"><span style="color: #073763; font-family: times new roman,serif; font-size: large;" data-mce-style="color: #073763; font-family: times new roman,serif; font-size: large;" color="#073763" face="times new roman, serif" size="4"><i><span style="line-height:15px"><a href="https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a" title="View public profile" name="UNIQUE_ID_SafeHtmlFilter_UNIQUE_ID_SafeHtmlFilter_UNIQUE_ID_SafeHtmlFilter_14e2c1de96f8c195_UNIQUE_ID_SafeHtmlFilter_SafeHtmlFilter_SafeHtmlFilter_webProfileURL" style="color:rgb(17,85,204);margin:0px;padding:0px;border-width:0px;outline:none;vertical-align:baseline;text-decoration:none" target="_blank">Linkedin</a></span><br>Junior Software Engineer @ <a href="http://www.daynix.com" target="_blank">Daynix</a>.</i></span></div></div></div></div></div> </div> </blockquote><div><br></div></div></body></html>