[Spice-commits] Branch '0.8' - 7 commits - client/red_channel.cpp client/red_client.cpp client/x11 common/marshaller.c server/smartcard.c

Christophe Fergau teuf at kemper.freedesktop.org
Tue Sep 20 07:09:50 PDT 2011


 client/red_channel.cpp    |   10 +++++++---
 client/red_client.cpp     |    1 +
 client/x11/platform.cpp   |    9 ++++++++-
 client/x11/red_window.cpp |   11 +++++++----
 common/marshaller.c       |   12 ++++++------
 server/smartcard.c        |   30 +++++++-----------------------
 6 files changed, 36 insertions(+), 37 deletions(-)

New commits:
commit d8e783905425ee5c26420f4d0e54d1905bae6609
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Wed Aug 10 12:11:43 2011 +0200

    fix infinite loop in resolution change
    
    After hours of investigation, I am a bit clueless.. It seems XRR is sending
    us spurious ScreenChangeNotify in a loop. So we keep calling
    init_monitors(), which creates new platform_win etc.. Although none of the
    clients seems to be resetting the screen (checked all XRRSet..). The fact
    that we create many platform_win looks like a bug to me, and indeed, it
    seems to help if we reuse the same platform_win over the various
    init_monitors() calls.
    
    Fixes rhbz #692833

diff --git a/client/x11/platform.cpp b/client/x11/platform.cpp
index 2adc160..7c74d38 100644
--- a/client/x11/platform.cpp
+++ b/client/x11/platform.cpp
@@ -81,7 +81,7 @@ static GLXFBConfig **fb_config = NULL;
 static XIM x_input_method = NULL;
 static XIC x_input_context = NULL;
 
-static Window platform_win;
+static Window platform_win = 0;
 static XContext win_proc_context;
 static ProcessLoop* main_loop = NULL;
 static int focus_count = 0;
@@ -922,6 +922,9 @@ DynamicScreen::DynamicScreen(Display* display, int screen, int& next_mon_id)
     , _saved_height (get_height())
     , _out_of_sync (false)
 {
+    if (platform_win != 0)
+        return;
+
     X_DEBUG_SYNC(display);
     //FIXME: replace RootWindow() in other refs as well?
     XLockDisplay(display);
@@ -1226,6 +1229,9 @@ MultyMonScreen::MultyMonScreen(Display* display, int screen, int& next_mon_id)
         throw;
     }
 
+    if (platform_win != 0)
+        return;
+
     XLockDisplay(display);
     platform_win = XCreateSimpleWindow(display, RootWindow(display, screen), 0, 0, 1, 1, 0, 0, 0);
     XUnlockDisplay(display);
commit d27a6708b00256d5ccccebfe38bb01cfdca8ec04
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Fri Aug 12 12:05:47 2011 +0200

    fix 2 X11 related leaks

diff --git a/client/x11/platform.cpp b/client/x11/platform.cpp
index 1facf73..2adc160 100644
--- a/client/x11/platform.cpp
+++ b/client/x11/platform.cpp
@@ -2958,6 +2958,7 @@ static unsigned int get_modifier_mask(KeySym modifier)
     XModifierKeymap* map = XGetModifierMapping(x_display);
     KeyCode keycode = XKeysymToKeycode(x_display, modifier);
     if (keycode == NoSymbol) {
+        XFreeModifiermap(map);
         return 0;
     }
 
diff --git a/client/x11/red_window.cpp b/client/x11/red_window.cpp
index d53a92f..cfc10c5 100644
--- a/client/x11/red_window.cpp
+++ b/client/x11/red_window.cpp
@@ -1306,16 +1306,19 @@ void RedWindow_p::move_to_current_desktop()
     unsigned long nitems_return;
     unsigned char *prop_return;
     long desktop = ~long(0);
+    int status;
 
     XLockDisplay(x_display);
-    if (XGetWindowProperty(x_display, root, wm_current_desktop, 0, 1, False, AnyPropertyType,
-                           &actual_type_return, &actual_format_return, &nitems_return,
-                           &bytes_after_return, &prop_return) == Success &&
-                                         actual_type_return != None && actual_format_return == 32) {
+    status = XGetWindowProperty(x_display, root, wm_current_desktop, 0, 1, False, AnyPropertyType,
+                                &actual_type_return, &actual_format_return, &nitems_return,
+                                &bytes_after_return, &prop_return);
+    if ((status  == Success) && (actual_type_return != None) && (actual_format_return == 32)) {
         desktop = *(uint32_t *)prop_return;
     } else {
         DBG(0, "get current desktop failed");
     }
+    if (status == Success)
+        XFree(prop_return);
     XUnlockDisplay(x_display);
 
     XEvent xevent;
commit 2cf6022100c97ee71df08e84efcc9a743f36544c
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Fri Aug 12 12:05:22 2011 +0200

    channel: fix EVP_PKEY leak

diff --git a/client/red_channel.cpp b/client/red_channel.cpp
index f4cdf52..fafb2e1 100644
--- a/client/red_channel.cpp
+++ b/client/red_channel.cpp
@@ -68,10 +68,7 @@ void RedChannelBase::link(uint32_t connection_id, const std::string& password,
     uint32_t link_res;
     uint32_t i;
 
-    EVP_PKEY *pubkey;
-    int nRSASize;
     BIO *bioKey;
-    RSA *rsa;
     uint8_t *buffer, *p;
     uint32_t expected_major;
 
@@ -168,6 +165,10 @@ void RedChannelBase::link(uint32_t connection_id, const std::string& password,
 
     bioKey = BIO_new(BIO_s_mem());
     if (bioKey != NULL) {
+        EVP_PKEY *pubkey;
+        int nRSASize;
+        RSA *rsa;
+
         BIO_write(bioKey, reply->pub_key, SPICE_TICKET_PUBKEY_BYTES);
         pubkey = d2i_PUBKEY_bio(bioKey, NULL);
         rsa = pubkey->pkey.rsa;
@@ -183,10 +184,13 @@ void RedChannelBase::link(uint32_t connection_id, const std::string& password,
                                rsa, RSA_PKCS1_OAEP_PADDING) > 0) {
             send((uint8_t*)bufEncrypted.get(), nRSASize);
         } else {
+            EVP_PKEY_free(pubkey);
+            BIO_free(bioKey);
             THROW("could not encrypt password");
         }
 
         memset(bufEncrypted.get(), 0, nRSASize);
+        EVP_PKEY_free(pubkey);
     } else {
         THROW("Could not initiate BIO");
     }
commit 97ebbc1c0be0464b1139ca8044a511d7516c75fa
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Fri Aug 12 10:32:56 2011 +0200

    always set VDAgentDisplayConfig::depth
    
    Even if VDAgentDisplayConfig::depth will be unused if the
    VD_AGENT_DISPLAY_CONFIG_FLAG_SET_COLOR_DEPTH isn't set, it's
    better to initialize it anyway to avoid warnings from valgrind.

diff --git a/client/red_client.cpp b/client/red_client.cpp
index 3de359d..d7a3e6a 100644
--- a/client/red_client.cpp
+++ b/client/red_client.cpp
@@ -737,6 +737,7 @@ void RedClient::send_agent_display_config()
         spice_marshaller_reserve_space(message->marshaller(), sizeof(VDAgentDisplayConfig));
 
     disp_config->flags = 0;
+    disp_config->depth = 0;
     if (_display_setting._disable_wallpaper) {
         disp_config->flags |= VD_AGENT_DISPLAY_CONFIG_FLAG_DISABLE_WALLPAPER;
     }
commit ff8c54d766a34c1a2dfa22707a2791e10e43c0be
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Wed Jul 27 17:20:13 2011 +0200

    fix integer marshalling helpers on big endian
    
    They were trying to convert the destination pointer to an integer before
    trying to dereference it. The initial conversion was meant to be a cast
    to a pointer of the right size, not to an integer.

diff --git a/common/marshaller.c b/common/marshaller.c
index 73bee4b..60cdacb 100644
--- a/common/marshaller.c
+++ b/common/marshaller.c
@@ -27,12 +27,12 @@
 #ifdef WORDS_BIGENDIAN
 #define write_int8(ptr,v) (*((int8_t *)(ptr)) = v)
 #define write_uint8(ptr,v) (*((uint8_t *)(ptr)) = v)
-#define write_int16(ptr,v) (*((int16_t)(ptr)) = SPICE_BYTESWAP16((uint16_t)(v)))
-#define write_uint16(ptr,v) (*((uint16_t)(ptr)) = SPICE_BYTESWAP16((uint16_t)(v)))
-#define write_int32(ptr,v) (*((int32_t)(ptr)) = SPICE_BYTESWAP32((uint32_t)(v)))
-#define write_uint32(ptr,v) (*((uint32_t)(ptr)) = SPICE_BYTESWAP32((uint32_t)(v)))
-#define write_int64(ptr,v) (*((int64_t)(ptr)) = SPICE_BYTESWAP64((uint64_t)(v)))
-#define write_uint64(ptr,v) (*((uint64_t)(ptr)) = SPICE_BYTESWAP64((uint64_t)(v)))
+#define write_int16(ptr,v) (*((int16_t *)(ptr)) = SPICE_BYTESWAP16((uint16_t)(v)))
+#define write_uint16(ptr,v) (*((uint16_t *)(ptr)) = SPICE_BYTESWAP16((uint16_t)(v)))
+#define write_int32(ptr,v) (*((int32_t *)(ptr)) = SPICE_BYTESWAP32((uint32_t)(v)))
+#define write_uint32(ptr,v) (*((uint32_t *)(ptr)) = SPICE_BYTESWAP32((uint32_t)(v)))
+#define write_int64(ptr,v) (*((int64_t *)(ptr)) = SPICE_BYTESWAP64((uint64_t)(v)))
+#define write_uint64(ptr,v) (*((uint64_t *)(ptr)) = SPICE_BYTESWAP64((uint64_t)(v)))
 #else
 #define write_int8(ptr,v) (*((int8_t *)(ptr)) = v)
 #define write_uint8(ptr,v) (*((uint8_t *)(ptr)) = v)
commit 3c03489a272c7d4505fc6c6dd18b0074b4731808
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Wed Jul 27 17:02:39 2011 +0200

    fix typo in big endian code path
    
    uint63_t should be uint64_t

diff --git a/common/marshaller.c b/common/marshaller.c
index 6ee7b6a..73bee4b 100644
--- a/common/marshaller.c
+++ b/common/marshaller.c
@@ -31,8 +31,8 @@
 #define write_uint16(ptr,v) (*((uint16_t)(ptr)) = SPICE_BYTESWAP16((uint16_t)(v)))
 #define write_int32(ptr,v) (*((int32_t)(ptr)) = SPICE_BYTESWAP32((uint32_t)(v)))
 #define write_uint32(ptr,v) (*((uint32_t)(ptr)) = SPICE_BYTESWAP32((uint32_t)(v)))
-#define write_int64(ptr,v) (*((int64_t)(ptr)) = SPICE_BYTESWAP64((uint63_t)(v)))
-#define write_uint64(ptr,v) (*((uint64_t)(ptr)) = SPICE_BYTESWAP64((uint63_t)(v)))
+#define write_int64(ptr,v) (*((int64_t)(ptr)) = SPICE_BYTESWAP64((uint64_t)(v)))
+#define write_uint64(ptr,v) (*((uint64_t)(ptr)) = SPICE_BYTESWAP64((uint64_t)(v)))
 #else
 #define write_int8(ptr,v) (*((int8_t *)(ptr)) = v)
 #define write_uint8(ptr,v) (*((uint8_t *)(ptr)) = v)
commit 506f03568297298eb7da1d904526bab373cceaca
Author: Alon Levy <alevy at redhat.com>
Date:   Wed Jul 20 16:36:36 2011 +0300

    server/smartcard: fix smartcard_channel_send_error
    
    It was sending the wrong data, the memory right after the VCSMsgHeader
    which was actually not where the data was.
    
    Fixed by having the header and data (VSCError, 4 bytes of the error code)
    embedded in the ErrorItem pipe item.

diff --git a/server/smartcard.c b/server/smartcard.c
index c75094d..8919400 100644
--- a/server/smartcard.c
+++ b/server/smartcard.c
@@ -41,8 +41,8 @@ enum {
 
 typedef struct ErrorItem {
     PipeItem base;
-    uint32_t reader_id;
-    uint32_t error;
+    VSCMsgHeader vheader;
+    VSCMsgError  error;
 } ErrorItem;
 
 typedef struct MsgItem {
@@ -288,23 +288,6 @@ static void smartcard_channel_send_data(RedChannel *channel, PipeItem *item, VSC
     red_channel_begin_send_message(channel);
 }
 
-static void smartcard_channel_send_message(RedChannel *channel, PipeItem *item,
-    uint32_t reader_id, VSCMsgType type, uint8_t* data, uint32_t len)
-{
-    VSCMsgHeader mhHeader;
-    //SpiceMarshaller* m = msg->marshaller();
-
-    mhHeader.type = type;
-    mhHeader.length = len;
-    mhHeader.reader_id = reader_id;
-    //_marshallers->msg_SpiceMsgData(m, &msgdata);
-    //spice_marshaller_add(m, (uint8_t*)&mhHeader, sizeof(mhHeader));
-    //spice_marshaller_add(m, data, len);
-    //marshaller_outgoing_write(msg);
-
-    smartcard_channel_send_data(channel, item, &mhHeader);
-}
-
 static void smartcard_channel_send_error(
     SmartCardChannel *smartcard_channel, PipeItem *item)
 {
@@ -312,8 +295,7 @@ static void smartcard_channel_send_error(
     VSCMsgError error;
 
     error.code = error_item->error;
-    smartcard_channel_send_message(&smartcard_channel->base, item, error_item->reader_id,
-        VSC_Error, (uint8_t*)&error, sizeof(error));
+    smartcard_channel_send_data(&smartcard_channel->base, item, &error_item->vheader);
 }
 
 static void smartcard_channel_send_msg(
@@ -367,8 +349,10 @@ static void smartcard_push_error(SmartCardChannel* channel, uint32_t reader_id,
     ErrorItem *error_item = spice_new0(ErrorItem, 1);
 
     error_item->base.type = PIPE_ITEM_TYPE_ERROR;
-    error_item->reader_id = reader_id;
-    error_item->error = error;
+    error_item->vheader.reader_id = reader_id;
+    error_item->vheader.type = VSC_Error;
+    error_item->vheader.length = sizeof(error_item->error);
+    error_item->error.code = error;
     smartcard_channel_pipe_add(channel, &error_item->base);
 }
 


More information about the Spice-commits mailing list