GUACAMOLE-422: Remove duplicate code and migrate handshake to user handlers.

src/libguac/guacamole/user.h

@@ -505,79 +505,6 @@ struct guac_user {
 
 };
 
-/**
- * Handler for Guacamole protocol opcode specific to the handshake that
- * happens between client and server at the beginning of the connection.  The
- * handler will be invoked when the matching opcode is received during the
- * handshake process.
- * 
- * @param user
- *     The user that initiated the handshake.
- * 
- * @param parser
- *     The parser allocated for parsing the data provided by the client.
- * 
- * @param timeout
- *     The timeout, in microseconds, for parsing the value.
- * 
- * @return
- *     Zero if the handshake instruction is successfully parsed; otherwise
- *     false.
- */
-typedef int __guac_handshake_handler(guac_user* user, int argc, char** argv);
-
-/**
- * Structure that maps opcodes received during the handshake phase of the
- * connection to callback functions used when those opcodes are received.
- */
-typedef struct __guac_handshake_mapping {
-    
-    /**
-     * The instruction opcode which maps to the handler.
-     */
-    char* opcode;
-    
-    /**
-     * The handler function used when specified opcode is received.
-     */
-    __guac_handshake_handler* handler;
-    
-} __guac_handshake_mapping;
-
-/**
- * Internal handler function that is called when the size instruction is
- * received during the handshake process.
- */
-__guac_handshake_handler __guac_handshake_size_handler;
-
-/**
- * Internal handler function that is called when the audio instruction is
- * received during the handshake process, specifying the audio mimetypes
- * available to the client.
- */
-__guac_handshake_handler __guac_handshake_audio_handler;
-
-/**
- * Internal handler function that is called when the video instruction is
- * received during the handshake process, specifying the video mimetypes
- * available to the client.
- */
-__guac_handshake_handler __guac_handshake_video_handler;
-
-/**
- * Internal handler function that is called when the image instruction is
- * received during the handshake process, specifying the image mimetypes
- * available to the client.
- */
-__guac_handshake_handler __guac_handshake_image_handler;
-
-/**
- * Internal handler function that is called when the timezone instruction is
- * received during the handshake process, specifying the timezone of the
- * client.
- */
-__guac_handshake_handler __guac_handshake_timezone_handler;
-
 /**
  * Allocates a new, blank user, not associated with any specific client or
  * socket.
@@ -617,32 +544,6 @@ void guac_user_free(guac_user* user);
  */
 int guac_user_handle_connection(guac_user* user, int usec_timeout);
 
-/**
- * Call the appropriate handler defined by the given user for the given
- * instruction. A comparison is made between the instruction opcode and the
- * initial handler lookup table defined in user-handlers.c. The intial handlers
- * will in turn call the user's handler (if defined).
- *
- * @param user
- *     The user whose handlers should be called.
- *
- * @param opcode
- *     The opcode of the instruction to pass to the user via the appropriate
- *     handler.
- *
- * @param argc
- *     The number of arguments which are part of the instruction.
- *
- * @param argv
- *     An array of all arguments which are part of the instruction.
- *
- * @return
- *     Non-negative if the instruction was handled successfully, or negative
- *     if an error occurred.
- */
-int guac_user_handle_instruction(guac_user* user, const char* opcode,
-        int argc, char** argv);
-
 /**
  * Allocates a new stream. An arbitrary index is automatically assigned
  * if no previously-allocated stream is available for use.

src/libguac/user-handlers.c

@@ -31,6 +31,7 @@
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
+#include <string.h>
 
 /* Guacamole instruction handler map */
 
@@ -53,6 +54,17 @@ __guac_instruction_handler_mapping __guac_instruction_handler_map[] = {
    {NULL,         NULL}
 };
 
+/* Guacamole handshake handler map */
+
+__guac_instruction_handler_mapping __guac_handshake_handler_map[] = {
+    {"size",     __guac_handshake_size_handler},
+    {"audio",    __guac_handshake_audio_handler},
+    {"video",    __guac_handshake_video_handler},
+    {"image",    __guac_handshake_image_handler},
+    {"timezone", __guac_handshake_timezone_handler},
+    {NULL,       NULL}
+};
+
 /**
  * Parses a 64-bit integer from the given string. It is assumed that the string
  * will contain only decimal digits, with an optional leading minus sign.
@@ -581,3 +593,122 @@ int __guac_handle_disconnect(guac_user* user, int argc, char** argv) {
     return 0;
 }
 
+/* Guacamole handshake handler functions. */
+
+int __guac_handshake_size_handler(guac_user* user, int argc, char** argv) {
+    
+    /* Validate size of instruction. */
+    if (argc < 2) {
+        guac_user_log(user, GUAC_LOG_ERROR, "Received \"size\" "
+                "instruction lacked required arguments.");
+        return 1;
+    }
+    
+    /* Parse optimal screen dimensions from size instruction */
+    user->info.optimal_width  = atoi(argv[0]);
+    user->info.optimal_height = atoi(argv[1]);
+
+    /* If DPI given, set the user resolution */
+    if (argc >= 3)
+        user->info.optimal_resolution = atoi(argv[2]);
+
+    /* Otherwise, use a safe default for rough backwards compatibility */
+    else
+        user->info.optimal_resolution = 96;
+    
+    return 0;
+    
+}
+
+int __guac_handshake_audio_handler(guac_user* user, int argc, char** argv) {
+
+    /* Store audio mimetypes */
+    user->info.audio_mimetypes = (const char**) guac_copy_mimetypes(argv, argc);
+    
+    return 0;
+    
+}
+
+int __guac_handshake_video_handler(guac_user* user, int argc, char** argv) {
+
+    /* Store video mimetypes */
+    user->info.video_mimetypes = (const char**) guac_copy_mimetypes(argv, argc);
+    
+    return 0;
+    
+}
+
+int __guac_handshake_image_handler(guac_user* user, int argc, char** argv) {
+    
+    /* Store image mimetypes */
+    user->info.image_mimetypes = (const char**) guac_copy_mimetypes(argv, argc);
+    
+    return 0;
+    
+}
+
+int __guac_handshake_timezone_handler(guac_user* user, int argc, char** argv) {
+    
+    /* Free any past value */
+    free((char *) user->info.timezone);
+    
+    /* Store timezone, if present */
+    if (argc > 0 && strcmp(argv[0], ""))
+        user->info.timezone = (const char*) strdup(argv[0]);
+    
+    return 0;
+    
+}
+
+char** guac_copy_mimetypes(char** mimetypes, int count) {
+
+    int i;
+
+    /* Allocate sufficient space for NULL-terminated array of mimetypes */
+    char** mimetypes_copy = malloc(sizeof(char*) * (count+1));
+
+    /* Copy each provided mimetype */
+    for (i = 0; i < count; i++)
+        mimetypes_copy[i] = strdup(mimetypes[i]);
+
+    /* Terminate with NULL */
+    mimetypes_copy[count] = NULL;
+
+    return mimetypes_copy;
+
+}
+
+void guac_free_mimetypes(char** mimetypes) {
+
+    char** current_mimetype = mimetypes;
+
+    /* Free all strings within NULL-terminated mimetype array */
+    while (*current_mimetype != NULL) {
+        free(*current_mimetype);
+        current_mimetype++;
+    }
+
+    /* Free the array itself, now that its contents have been freed */
+    free(mimetypes);
+
+}
+
+int guac_user_handle_instruction(__guac_instruction_handler_mapping* map,
+        guac_user* user, const char* opcode, int argc, char** argv) {
+
+    /* For each defined instruction */
+    __guac_instruction_handler_mapping* current = map;
+    while (current->opcode != NULL) {
+
+        /* If recognized, call handler */
+        if (strcmp(opcode, current->opcode) == 0)
+            return current->handler(user, argc, argv);
+
+        current++;
+    }
+
+    /* If unrecognized, ignore */
+    return 0;
+
+}
+

src/libguac/user-handlers.h

@@ -177,6 +177,40 @@ __guac_instruction_handler __guac_handle_size;
  */
 __guac_instruction_handler __guac_handle_disconnect;
 
+/**
+ * Internal handler function that is called when the size instruction is
+ * received during the handshake process.
+ */
+__guac_instruction_handler __guac_handshake_size_handler;
+
+/**
+ * Internal handler function that is called when the audio instruction is
+ * received during the handshake process, specifying the audio mimetypes
+ * available to the client.
+ */
+__guac_instruction_handler __guac_handshake_audio_handler;
+
+/**
+ * Internal handler function that is called when the video instruction is
+ * received during the handshake process, specifying the video mimetypes
+ * available to the client.
+ */
+__guac_instruction_handler __guac_handshake_video_handler;
+
+/**
+ * Internal handler function that is called when the image instruction is
+ * received during the handshake process, specifying the image mimetypes
+ * available to the client.
+ */
+__guac_instruction_handler __guac_handshake_image_handler;
+
+/**
+ * Internal handler function that is called when the timezone instruction is
+ * received during the handshake process, specifying the timezone of the
+ * client.
+ */
+__guac_instruction_handler __guac_handshake_timezone_handler;
+
 /**
  * Instruction handler mapping table. This is a NULL-terminated array of
  * __guac_instruction_handler_mapping structures, each mapping an opcode
@@ -186,4 +220,71 @@ __guac_instruction_handler __guac_handle_disconnect;
  */
 extern __guac_instruction_handler_mapping __guac_instruction_handler_map[];
 
+/**
+ * Handler mapping table for instructions (opcodes) specifically for the
+ * handshake portion of the connection.  Each
+ * __guac_instruction_handler_mapping structure within this NULL-terminated
+ * array maps an opcode to a __guac_instruction_handler.  The end of the array
+ * must be marked with a mapping with the opcode set to NULL.
+ */
+extern __guac_instruction_handler_mapping __guac_handshake_handler_map[];
+
+/**
+ * Frees the given array of mimetypes, including the space allocated to each
+ * mimetype string within the array. The provided array of mimetypes MUST have
+ * been allocated with guac_copy_mimetypes().
+ *
+ * @param mimetypes
+ *     The NULL-terminated array of mimetypes to free. This array MUST have
+ *     been previously allocated with guac_copy_mimetypes().
+ */
+void guac_free_mimetypes(char** mimetypes);
+
+/**
+ * Copies the given array of mimetypes (strings) into a newly-allocated NULL-
+ * terminated array of strings. Both the array and the strings within the array
+ * are newly-allocated and must be later freed via guac_free_mimetypes().
+ *
+ * @param mimetypes
+ *     The array of mimetypes to copy.
+ *
+ * @param count
+ *     The number of mimetypes in the given array.
+ *
+ * @return
+ *     A newly-allocated, NULL-terminated array containing newly-allocated
+ *     copies of each of the mimetypes provided in the original mimetypes
+ *     array.
+ */
+char** guac_copy_mimetypes(char** mimetypes, int count);
+
+/**
+ * Call the appropriate handler defined by the given user for the given
+ * instruction. A comparison is made between the instruction opcode and the
+ * initial handler lookup table defined in user-handlers.c. The intial handlers
+ * will in turn call the user's handler (if defined).
+ *
+ * @param map
+ *     The array that holds the opcode to handler mappings.
+ * 
+ * @param user
+ *     The user whose handlers should be called.
+ *
+ * @param opcode
+ *     The opcode of the instruction to pass to the user via the appropriate
+ *     handler.
+ *
+ * @param argc
+ *     The number of arguments which are part of the instruction.
+ *
+ * @param argv
+ *     An array of all arguments which are part of the instruction.
+ *
+ * @return
+ *     Non-negative if the instruction was handled successfully, or negative
+ *     if an error occurred.
+ */
+int guac_user_handle_instruction(__guac_instruction_handler_mapping* map,
+        guac_user* user, const char* opcode, int argc, char** argv);
+
 #endif

src/libguac/user-handshake.c

@@ -113,141 +113,6 @@ static void guac_user_log_handshake_failure(guac_user* user) {
 
 }
 
-/**
- * Copies the given array of mimetypes (strings) into a newly-allocated NULL-
- * terminated array of strings. Both the array and the strings within the array
- * are newly-allocated and must be later freed via guac_free_mimetypes().
- *
- * @param mimetypes
- *     The array of mimetypes to copy.
- *
- * @param count
- *     The number of mimetypes in the given array.
- *
- * @return
- *     A newly-allocated, NULL-terminated array containing newly-allocated
- *     copies of each of the mimetypes provided in the original mimetypes
- *     array.
- */
-static char** guac_copy_mimetypes(char** mimetypes, int count) {
-
-    int i;
-
-    /* Allocate sufficient space for NULL-terminated array of mimetypes */
-    char** mimetypes_copy = malloc(sizeof(char*) * (count+1));
-
-    /* Copy each provided mimetype */
-    for (i = 0; i < count; i++)
-        mimetypes_copy[i] = strdup(mimetypes[i]);
-
-    /* Terminate with NULL */
-    mimetypes_copy[count] = NULL;
-
-    return mimetypes_copy;
-
-}
-
-/**
- * Frees the given array of mimetypes, including the space allocated to each
- * mimetype string within the array. The provided array of mimetypes MUST have
- * been allocated with guac_copy_mimetypes().
- *
- * @param mimetypes
- *     The NULL-terminated array of mimetypes to free. This array MUST have
- *     been previously allocated with guac_copy_mimetypes().
- */
-static void guac_free_mimetypes(char** mimetypes) {
-
-    char** current_mimetype = mimetypes;
-
-    /* Free all strings within NULL-terminated mimetype array */
-    while (*current_mimetype != NULL) {
-        free(*current_mimetype);
-        current_mimetype++;
-    }
-
-    /* Free the array itself, now that its contents have been freed */
-    free(mimetypes);
-
-}
-
-/* Guacamole handshake handler functions. */
-
-int __guac_handshake_size_handler(guac_user* user, int argc, char** argv) {
-    
-    /* Validate size of instruction. */
-    if (argc < 2) {
-        guac_user_log(user, GUAC_LOG_ERROR, "Received \"size\" "
-                "instruction lacked required arguments.");
-        return 1;
-    }
-    
-    /* Parse optimal screen dimensions from size instruction */
-    user->info.optimal_width  = atoi(argv[0]);
-    user->info.optimal_height = atoi(argv[1]);
-
-    /* If DPI given, set the user resolution */
-    if (argc >= 3)
-        user->info.optimal_resolution = atoi(argv[2]);
-
-    /* Otherwise, use a safe default for rough backwards compatibility */
-    else
-        user->info.optimal_resolution = 96;
-    
-    return 0;
-    
-}
-
-int __guac_handshake_audio_handler(guac_user* user, int argc, char** argv) {
-
-    /* Store audio mimetypes */
-    user->info.audio_mimetypes = (const char**) guac_copy_mimetypes(argv, argc);
-    
-    return 0;
-    
-}
-
-int __guac_handshake_video_handler(guac_user* user, int argc, char** argv) {
-
-    /* Store video mimetypes */
-    user->info.video_mimetypes = (const char**) guac_copy_mimetypes(argv, argc);
-    
-    return 0;
-    
-}
-
-int __guac_handshake_image_handler(guac_user* user, int argc, char** argv) {
-    
-    /* Store image mimetypes */
-    user->info.image_mimetypes = (const char**) guac_copy_mimetypes(argv, argc);
-    
-    return 0;
-    
-}
-
-int __guac_handshake_timezone_handler(guac_user* user, int argc, char** argv) {
-    
-    /* Free any past value */
-    free((char *) user->info.timezone);
-    
-    /* Store timezone, if present */
-    if (argc > 0 && strcmp(argv[0], ""))
-        user->info.timezone = (const char*) strdup(argv[0]);
-    
-    return 0;
-    
-}
-
-/* Guacamole handshake handler mappings. */
-__guac_handshake_mapping __guac_handshake_map[] = {
-    {"size",     __guac_handshake_size_handler},
-    {"audio",    __guac_handshake_audio_handler},
-    {"video",    __guac_handshake_video_handler},
-    {"image",    __guac_handshake_image_handler},
-    {"timezone", __guac_handshake_timezone_handler},
-    {NULL,       NULL}
-};
-
 /**
  * The thread which handles all user input, calling event handlers for received
  * instructions.
@@ -296,7 +161,8 @@ static void* guac_user_input_thread(void* data) {
         guac_error_message = NULL;
 
         /* Call handler, stop on error */
-        if (guac_user_handle_instruction(user, parser->opcode, parser->argc, parser->argv) < 0) {
+        if (guac_user_handle_instruction(__guac_instruction_handler_map, 
+                user, parser->opcode, parser->argc, parser->argv) < 0) {
 
             /* Log error */
             guac_user_log_guac_error(user, GUAC_LOG_WARNING,
@@ -399,39 +265,21 @@ int guac_user_handle_connection(guac_user* user, int usec_timeout) {
         guac_user_log(user, GUAC_LOG_DEBUG, "Processing instruction: %s",
                 parser->opcode);
         
-        /* Loop available opcodes and run handler if/when match found. */
+        /* Run instruction handler for opcode with arguments. */
-        __guac_handshake_mapping* current = __guac_handshake_map;
+        if (guac_user_handle_instruction(__guac_handshake_handler_map, user,
-        while (current->opcode != NULL) {
+                parser->opcode, parser->argc, parser->argv)) {
             
-            /* Check if loop opcode matches parsed opcode. */
+            guac_user_log_handshake_failure(user);
-            if (strcmp(parser->opcode, current->opcode) == 0) {
+            guac_user_log_guac_error(user, GUAC_LOG_DEBUG,
+                    "Error handling handling opcode during handshake.");
+            guac_user_log(user, GUAC_LOG_DEBUG, "Failed opcode: %s",
+                    parser->opcode);
 
-                    /* If calling the handler fails, log it and return. */
+            guac_parser_free(parser);
-                    if (current->handler(user, parser->argc, parser->argv)) {
+            return 1;
             
-                        guac_user_log_handshake_failure(user);
-                        guac_user_log_guac_error(user, GUAC_LOG_DEBUG,
-                                "Error handling handling opcode during handshake.");
-                        guac_user_log(user, GUAC_LOG_DEBUG, "Failed opcode: %s",
-                                current->opcode);
-
-                        guac_parser_free(parser);
-                        return 1;
-                    }
-                    
-                    /* If calling the handler has succeeded, log it and break. */
-                    else {
-                        guac_user_log(user, GUAC_LOG_DEBUG,
-                                "Successfully processed instruction: \"%s\"",
-                                current->opcode);
-                        break;
-                    }
-
-            }
-            
-            /* Move to next opcode. */
-            current++;
         }
+        
     }
 
     /* Acknowledge connection availability */

src/libguac/user.c

@@ -167,24 +167,6 @@ void guac_user_free_object(guac_user* user, guac_object* object) {
 
 }
 
-int guac_user_handle_instruction(guac_user* user, const char* opcode, int argc, char** argv) {
-
-    /* For each defined instruction */
-    __guac_instruction_handler_mapping* current = __guac_instruction_handler_map;
-    while (current->opcode != NULL) {
-
-        /* If recognized, call handler */
-        if (strcmp(opcode, current->opcode) == 0)
-            return current->handler(user, argc, argv);
-
-        current++;
-    }
-
-    /* If unrecognized, ignore */
-    return 0;
-
-}
-
 void guac_user_stop(guac_user* user) {
     user->active = 0;
 }