Patchwork Remove magic numbers for VNC message IDs from code

login
register
mail settings
Submitter Daniel P. Berrange
Date March 31, 2010, 4:45 p.m.
Message ID <1270053957-25898-1-git-send-email-berrange@redhat.com>
Download mbox | patch
Permalink /patch/49166/
State New
Headers show

Comments

Daniel P. Berrange - March 31, 2010, 4:45 p.m.
The code processing incoming & sending outgoing messages from/to
clients used embedded magic numbers for all message IDs. This
made the code a little hard to follow. Add constants in the vnc.h
header file for all message IDs and use them in the code

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 vnc.c |   56 ++++++++++++++++++++++++++++----------------------------
 vnc.h |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 28 deletions(-)
Anthony Liguori - March 31, 2010, 5:07 p.m.
On 03/31/2010 11:45 AM, Daniel P. Berrange wrote:
> diff --git a/vnc.h b/vnc.h
> index 0a7487b..8752de0 100644
> --- a/vnc.h
> +++ b/vnc.h
> @@ -276,6 +276,57 @@ enum {
>   #define VNC_FEATURE_COPYRECT_MASK            (1<<  VNC_FEATURE_COPYRECT)
>
>
> +/* Client ->  Server message IDs */
> +#define VNC_MSG_CLIENT_SET_PIXEL_FORMAT           0
> +#define VNC_MSG_CLIENT_SET_ENCODINGS              2
> +#define VNC_MSG_CLIENT_FRAMEBUFFER_UPDATE_REQUEST 3
> +#define VNC_MSG_CLIENT_KEY_EVENT                  4
> +#define VNC_MSG_CLIENT_POINTER_EVENT              5
> +#define VNC_MSG_CLIENT_CUT_TEXT                   6
> +#define VNC_MSG_CLIENT_VMWARE_0                   127
> +#define VNC_MSG_CLIENT_CALL_CONTROL               249
> +#define VNC_MSG_CLIENT_XVP                        250
> +#define VNC_MSG_CLIENT_SET_DESKTOP_SIZE           251
> +#define VNC_MSG_CLIENT_TIGHT                      252
> +#define VNC_MSG_CLIENT_GII                        253
> +#define VNC_MSG_CLIENT_VMWARE_1                   254
> +#define VNC_MSG_CLIENT_ALIGUORI                   255
> +
> +/* Server ->  Client message IDs */
> +#define VNC_MSG_SERVER_FRAMEBUFFER_UPDATE         0
> +#define VNC_MSG_SERVER_SET_COLOUR_MAP_ENTRIES     1
> +#define VNC_MSG_SERVER_BELL                       2
> +#define VNC_MSG_SERVER_CUT_TEXT                   3
> +#define VNC_MSG_SERVER_VMWARE_0                   127
> +#define VNC_MSG_SERVER_CALL_CONTROL               249
> +#define VNC_MSG_SERVER_XVP                        250
> +#define VNC_MSG_SERVER_TIGHT                      252
> +#define VNC_MSG_SERVER_GII                        253
> +#define VNC_MSG_SERVER_VMWARE_1                   254
> +#define VNC_MSG_SERVER_ALIGUORI                   255
> +
> +
> +
> +/* Aliguori client ->  server message IDs */
> +#define VNC_MSG_CLIENT_ALIGUORI_EXT_KEY_EVENT     0
> +#define VNC_MSG_CLIENT_ALIGUORI_AUDIO             1
> +
> +/* Aliguori server ->  client message IDs */
> +#define VNC_MSG_SERVER_ALIGUORI_AUDIO             1
> +
> +
> +
> +/* Aliguori client ->  server audio message IDs */
> +#define VNC_MSG_CLIENT_ALIGUORI_AUDIO_ENABLE      0
> +#define VNC_MSG_CLIENT_ALIGUORI_AUDIO_DISABLE     1
> +#define VNC_MSG_CLIENT_ALIGUORI_AUDIO_SET_FORMAT  2
> +
> +/* Aliguori server ->  client audio message IDs */
> +#define VNC_MSG_SERVER_ALIGUORI_AUDIO_END         0
> +#define VNC_MSG_SERVER_ALIGUORI_AUDIO_BEGIN       1
> +#define VNC_MSG_SERVER_ALIGUORI_AUDIO_DATA        2
>    

Nice patch, but let's s/aliguori/qemu/.

Regards,

Anthony Liguori

> +
>   /*****************************************************************************
>    *
>    * Internal APIs
>
Daniel P. Berrange - March 31, 2010, 5:11 p.m.
On Wed, Mar 31, 2010 at 12:07:51PM -0500, Anthony Liguori wrote:
> On 03/31/2010 11:45 AM, Daniel P. Berrange wrote:
> >diff --git a/vnc.h b/vnc.h
> >index 0a7487b..8752de0 100644
> >--- a/vnc.h
> >+++ b/vnc.h
> >@@ -276,6 +276,57 @@ enum {
> >  #define VNC_FEATURE_COPYRECT_MASK            (1<<  VNC_FEATURE_COPYRECT)
> >
> >
> >+/* Client ->  Server message IDs */
> >+#define VNC_MSG_CLIENT_SET_PIXEL_FORMAT           0
> >+#define VNC_MSG_CLIENT_SET_ENCODINGS              2
> >+#define VNC_MSG_CLIENT_FRAMEBUFFER_UPDATE_REQUEST 3
> >+#define VNC_MSG_CLIENT_KEY_EVENT                  4
> >+#define VNC_MSG_CLIENT_POINTER_EVENT              5
> >+#define VNC_MSG_CLIENT_CUT_TEXT                   6
> >+#define VNC_MSG_CLIENT_VMWARE_0                   127
> >+#define VNC_MSG_CLIENT_CALL_CONTROL               249
> >+#define VNC_MSG_CLIENT_XVP                        250
> >+#define VNC_MSG_CLIENT_SET_DESKTOP_SIZE           251
> >+#define VNC_MSG_CLIENT_TIGHT                      252
> >+#define VNC_MSG_CLIENT_GII                        253
> >+#define VNC_MSG_CLIENT_VMWARE_1                   254
> >+#define VNC_MSG_CLIENT_ALIGUORI                   255
> >+
> >+/* Server ->  Client message IDs */
> >+#define VNC_MSG_SERVER_FRAMEBUFFER_UPDATE         0
> >+#define VNC_MSG_SERVER_SET_COLOUR_MAP_ENTRIES     1
> >+#define VNC_MSG_SERVER_BELL                       2
> >+#define VNC_MSG_SERVER_CUT_TEXT                   3
> >+#define VNC_MSG_SERVER_VMWARE_0                   127
> >+#define VNC_MSG_SERVER_CALL_CONTROL               249
> >+#define VNC_MSG_SERVER_XVP                        250
> >+#define VNC_MSG_SERVER_TIGHT                      252
> >+#define VNC_MSG_SERVER_GII                        253
> >+#define VNC_MSG_SERVER_VMWARE_1                   254
> >+#define VNC_MSG_SERVER_ALIGUORI                   255
> >+
> >+
> >+
> >+/* Aliguori client ->  server message IDs */
> >+#define VNC_MSG_CLIENT_ALIGUORI_EXT_KEY_EVENT     0
> >+#define VNC_MSG_CLIENT_ALIGUORI_AUDIO             1
> >+
> >+/* Aliguori server ->  client message IDs */
> >+#define VNC_MSG_SERVER_ALIGUORI_AUDIO             1
> >+
> >+
> >+
> >+/* Aliguori client ->  server audio message IDs */
> >+#define VNC_MSG_CLIENT_ALIGUORI_AUDIO_ENABLE      0
> >+#define VNC_MSG_CLIENT_ALIGUORI_AUDIO_DISABLE     1
> >+#define VNC_MSG_CLIENT_ALIGUORI_AUDIO_SET_FORMAT  2
> >+
> >+/* Aliguori server ->  client audio message IDs */
> >+#define VNC_MSG_SERVER_ALIGUORI_AUDIO_END         0
> >+#define VNC_MSG_SERVER_ALIGUORI_AUDIO_BEGIN       1
> >+#define VNC_MSG_SERVER_ALIGUORI_AUDIO_DATA        2
> >   
> 
> Nice patch, but let's s/aliguori/qemu/.

I was just matching the naming from the RFB protocol spec :-) I'll
send an updated patch...

Regards,
Daniel

Patch

diff --git a/vnc.c b/vnc.c
index e678fcc..6095696 100644
--- a/vnc.c
+++ b/vnc.c
@@ -541,7 +541,7 @@  static void vnc_dpy_resize(DisplayState *ds)
         vnc_colordepth(vs);
         if (size_changed) {
             if (vs->csock != -1 && vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
-                vnc_write_u8(vs, 0);  /* msg id */
+                vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
                 vnc_write_u8(vs, 0);
                 vnc_write_u16(vs, 1); /* number of rects */
                 vnc_framebuffer_update(vs, 0, 0, ds_get_width(ds), ds_get_height(ds),
@@ -844,7 +844,7 @@  static void send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
 static void vnc_copy(VncState *vs, int src_x, int src_y, int dst_x, int dst_y, int w, int h)
 {
     /* send bitblit op to the vnc client */
-    vnc_write_u8(vs, 0);  /* msg id */
+    vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
     vnc_write_u8(vs, 0);
     vnc_write_u16(vs, 1); /* number of rects */
     vnc_framebuffer_update(vs, dst_x, dst_y, w, h, VNC_ENCODING_COPYRECT);
@@ -964,7 +964,7 @@  static int vnc_update_client(VncState *vs, int has_dirty)
          * send them to the client.
          */
         n_rectangles = 0;
-        vnc_write_u8(vs, 0);  /* msg id */
+        vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
         vnc_write_u8(vs, 0);
         saved_offset = vs->output.offset;
         vnc_write_u16(vs, 0);
@@ -1013,16 +1013,16 @@  static void audio_capture_notify(void *opaque, audcnotification_e cmd)
 
     switch (cmd) {
     case AUD_CNOTIFY_DISABLE:
-        vnc_write_u8(vs, 255);
-        vnc_write_u8(vs, 1);
-        vnc_write_u16(vs, 0);
+        vnc_write_u8(vs, VNC_MSG_SERVER_ALIGUORI);
+        vnc_write_u8(vs, VNC_MSG_SERVER_ALIGUORI_AUDIO);
+        vnc_write_u16(vs, VNC_MSG_SERVER_ALIGUORI_AUDIO_END);
         vnc_flush(vs);
         break;
 
     case AUD_CNOTIFY_ENABLE:
-        vnc_write_u8(vs, 255);
-        vnc_write_u8(vs, 1);
-        vnc_write_u16(vs, 1);
+        vnc_write_u8(vs, VNC_MSG_SERVER_ALIGUORI);
+        vnc_write_u8(vs, VNC_MSG_SERVER_ALIGUORI_AUDIO);
+        vnc_write_u16(vs, VNC_MSG_SERVER_ALIGUORI_AUDIO_BEGIN);
         vnc_flush(vs);
         break;
     }
@@ -1036,9 +1036,9 @@  static void audio_capture(void *opaque, void *buf, int size)
 {
     VncState *vs = opaque;
 
-    vnc_write_u8(vs, 255);
-    vnc_write_u8(vs, 1);
-    vnc_write_u16(vs, 2);
+    vnc_write_u8(vs, VNC_MSG_SERVER_ALIGUORI);
+    vnc_write_u8(vs, VNC_MSG_SERVER_ALIGUORI_AUDIO);
+    vnc_write_u16(vs, VNC_MSG_SERVER_ALIGUORI_AUDIO_DATA);
     vnc_write_u32(vs, size);
     vnc_write(vs, buf, size);
     vnc_flush(vs);
@@ -1434,7 +1434,7 @@  static void check_pointer_type_change(Notifier *notifier)
     int absolute = kbd_mouse_is_absolute();
 
     if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE) && vs->absolute != absolute) {
-        vnc_write_u8(vs, 0);
+        vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
         vnc_write_u8(vs, 0);
         vnc_write_u16(vs, 1);
         vnc_framebuffer_update(vs, absolute, 0,
@@ -1747,7 +1747,7 @@  static void framebuffer_update_request(VncState *vs, int incremental,
 
 static void send_ext_key_event_ack(VncState *vs)
 {
-    vnc_write_u8(vs, 0);
+    vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
     vnc_write_u8(vs, 0);
     vnc_write_u16(vs, 1);
     vnc_framebuffer_update(vs, 0, 0, ds_get_width(vs->ds), ds_get_height(vs->ds),
@@ -1757,7 +1757,7 @@  static void send_ext_key_event_ack(VncState *vs)
 
 static void send_ext_audio_ack(VncState *vs)
 {
-    vnc_write_u8(vs, 0);
+    vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
     vnc_write_u8(vs, 0);
     vnc_write_u16(vs, 1);
     vnc_framebuffer_update(vs, 0, 0, ds_get_width(vs->ds), ds_get_height(vs->ds),
@@ -1930,7 +1930,7 @@  static void vnc_colordepth(VncState *vs)
 {
     if (vnc_has_feature(vs, VNC_FEATURE_WMVI)) {
         /* Sending a WMVi message to notify the client*/
-        vnc_write_u8(vs, 0);  /* msg id */
+        vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
         vnc_write_u8(vs, 0);
         vnc_write_u16(vs, 1); /* number of rects */
         vnc_framebuffer_update(vs, 0, 0, ds_get_width(vs->ds), 
@@ -1955,7 +1955,7 @@  static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
     }
 
     switch (data[0]) {
-    case 0:
+    case VNC_MSG_CLIENT_SET_PIXEL_FORMAT:
         if (len == 1)
             return 20;
 
@@ -1965,7 +1965,7 @@  static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
                          read_u16(data, 12), read_u8(data, 14),
                          read_u8(data, 15), read_u8(data, 16));
         break;
-    case 2:
+    case VNC_MSG_CLIENT_SET_ENCODINGS:
         if (len == 1)
             return 4;
 
@@ -1983,7 +1983,7 @@  static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
 
         set_encodings(vs, (int32_t *)(data + 4), limit);
         break;
-    case 3:
+    case VNC_MSG_CLIENT_FRAMEBUFFER_UPDATE_REQUEST:
         if (len == 1)
             return 10;
 
@@ -1991,19 +1991,19 @@  static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
                                    read_u8(data, 1), read_u16(data, 2), read_u16(data, 4),
                                    read_u16(data, 6), read_u16(data, 8));
         break;
-    case 4:
+    case VNC_MSG_CLIENT_KEY_EVENT:
         if (len == 1)
             return 8;
 
         key_event(vs, read_u8(data, 1), read_u32(data, 4));
         break;
-    case 5:
+    case VNC_MSG_CLIENT_POINTER_EVENT:
         if (len == 1)
             return 6;
 
         pointer_event(vs, read_u8(data, 1), read_u16(data, 2), read_u16(data, 4));
         break;
-    case 6:
+    case VNC_MSG_CLIENT_CUT_TEXT:
         if (len == 1)
             return 8;
 
@@ -2015,30 +2015,30 @@  static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
 
         client_cut_text(vs, read_u32(data, 4), data + 8);
         break;
-    case 255:
+    case VNC_MSG_CLIENT_ALIGUORI:
         if (len == 1)
             return 2;
 
         switch (read_u8(data, 1)) {
-        case 0:
+        case VNC_MSG_CLIENT_ALIGUORI_EXT_KEY_EVENT:
             if (len == 2)
                 return 12;
 
             ext_key_event(vs, read_u16(data, 2),
                           read_u32(data, 4), read_u32(data, 8));
             break;
-        case 1:
+        case VNC_MSG_CLIENT_ALIGUORI_AUDIO:
             if (len == 2)
                 return 4;
 
             switch (read_u16 (data, 2)) {
-            case 0:
+            case VNC_MSG_CLIENT_ALIGUORI_AUDIO_ENABLE:
                 audio_add(vs);
                 break;
-            case 1:
+            case VNC_MSG_CLIENT_ALIGUORI_AUDIO_DISABLE:
                 audio_del(vs);
                 break;
-            case 2:
+            case VNC_MSG_CLIENT_ALIGUORI_AUDIO_SET_FORMAT:
                 if (len == 4)
                     return 10;
                 switch (read_u8(data, 4)) {
diff --git a/vnc.h b/vnc.h
index 0a7487b..8752de0 100644
--- a/vnc.h
+++ b/vnc.h
@@ -276,6 +276,57 @@  enum {
 #define VNC_FEATURE_COPYRECT_MASK            (1 << VNC_FEATURE_COPYRECT)
 
 
+/* Client -> Server message IDs */
+#define VNC_MSG_CLIENT_SET_PIXEL_FORMAT           0
+#define VNC_MSG_CLIENT_SET_ENCODINGS              2
+#define VNC_MSG_CLIENT_FRAMEBUFFER_UPDATE_REQUEST 3
+#define VNC_MSG_CLIENT_KEY_EVENT                  4
+#define VNC_MSG_CLIENT_POINTER_EVENT              5
+#define VNC_MSG_CLIENT_CUT_TEXT                   6
+#define VNC_MSG_CLIENT_VMWARE_0                   127
+#define VNC_MSG_CLIENT_CALL_CONTROL               249
+#define VNC_MSG_CLIENT_XVP                        250
+#define VNC_MSG_CLIENT_SET_DESKTOP_SIZE           251
+#define VNC_MSG_CLIENT_TIGHT                      252
+#define VNC_MSG_CLIENT_GII                        253
+#define VNC_MSG_CLIENT_VMWARE_1                   254
+#define VNC_MSG_CLIENT_ALIGUORI                   255
+
+/* Server -> Client message IDs */
+#define VNC_MSG_SERVER_FRAMEBUFFER_UPDATE         0
+#define VNC_MSG_SERVER_SET_COLOUR_MAP_ENTRIES     1
+#define VNC_MSG_SERVER_BELL                       2
+#define VNC_MSG_SERVER_CUT_TEXT                   3
+#define VNC_MSG_SERVER_VMWARE_0                   127
+#define VNC_MSG_SERVER_CALL_CONTROL               249
+#define VNC_MSG_SERVER_XVP                        250
+#define VNC_MSG_SERVER_TIGHT                      252
+#define VNC_MSG_SERVER_GII                        253
+#define VNC_MSG_SERVER_VMWARE_1                   254
+#define VNC_MSG_SERVER_ALIGUORI                   255
+
+
+
+/* Aliguori client -> server message IDs */
+#define VNC_MSG_CLIENT_ALIGUORI_EXT_KEY_EVENT     0
+#define VNC_MSG_CLIENT_ALIGUORI_AUDIO             1
+
+/* Aliguori server -> client message IDs */
+#define VNC_MSG_SERVER_ALIGUORI_AUDIO             1
+
+
+
+/* Aliguori client -> server audio message IDs */
+#define VNC_MSG_CLIENT_ALIGUORI_AUDIO_ENABLE      0
+#define VNC_MSG_CLIENT_ALIGUORI_AUDIO_DISABLE     1
+#define VNC_MSG_CLIENT_ALIGUORI_AUDIO_SET_FORMAT  2
+
+/* Aliguori server -> client audio message IDs */
+#define VNC_MSG_SERVER_ALIGUORI_AUDIO_END         0
+#define VNC_MSG_SERVER_ALIGUORI_AUDIO_BEGIN       1
+#define VNC_MSG_SERVER_ALIGUORI_AUDIO_DATA        2
+
+
 /*****************************************************************************
  *
  * Internal APIs