ottobackwards commented on a change in pull request #229:
URL: https://github.com/apache/plc4x/pull/229#discussion_r596083902



##########
File path: sandbox/plc4c/spi/src/system.c
##########
@@ -204,146 +204,71 @@ void plc4c_system_shutdown(plc4c_system *system) {}
 
 plc4c_return_code plc4c_system_create_connection(
     char *connection_string, plc4c_connection **connection) {
-  // Count the number of colons and question-marks so we know which pattern to
-  // use for matching and how large the arrays for containing the different
-  // segments should be.
-  int num_colons = 0;
-  int num_question_marks = 0;
-  char *protocol_code = NULL;
-  char *transport_code = NULL;
-  char *transport_connect_information = NULL;
-  char *parameters = NULL;
-  int start_segment_index = 0;
-  char *start_segment = connection_string;
-  // The connection string has two parts ... the first, where colons are the
-  // delimiters and the second where a question mark is the delimiter.
+
+  plc4c_connection *new_connection;
+  char connection_to_tokenize[strlen(connection_string) + 1];
+  char* connection_string_pos;
+  char* connection_token; 
+  char* parameters_token;
+  size_t i;
+
   enum mode {
     SEARCHING_FOR_COLONS,
     SEARCHING_FOR_QUESTION_MARKS
   } mode = SEARCHING_FOR_COLONS;
-  for (int i = 0; i <= strlen(connection_string); i++) {
-    // If we're in the first part of the connection string ... watch out for
-    // colons.
-    if (mode == SEARCHING_FOR_COLONS) {
-      // If we encounter a colon, depending on the number of colons already
-      // found, save the information in either the protocol code or transport
-      // code variable.
-      if (*(connection_string + i) == ':') {
-        num_colons++;
-        // The first colon delimits the protocol-code.
-        if (num_colons == 1) {
-          // Allocate enough memory to hold the sub-string.
-          protocol_code =
-              malloc(sizeof(char) * ((i - start_segment_index) + 1));
-          *(protocol_code + (i - start_segment_index)) = '\0';
-          // Copy the sub-string to the freshly allocated memory area.
-          strncpy(protocol_code, start_segment, (i - start_segment_index));
-
-          // Set the start of the next segment to directly after the colon.
-          start_segment_index = i + 1;
-          start_segment = connection_string + start_segment_index;
-
-          // If the following character would be a slash, we're probably
-          // finished and no transport code is provided. If this is the case,
-          // ensure it's actually a double-slash and if this is the case switch
-          // to the question-mark searching mode.
-          if (*start_segment == '/') {
-            if (*(start_segment + 1) == '/') {
-              mode = SEARCHING_FOR_QUESTION_MARKS;
-              start_segment_index += 2;
-              start_segment += 2;
-            } else {
-              return INVALID_CONNECTION_STRING;
-            }
-          }
-        }
-        // If we encountered a second colon, this is the transport code.
-        else if (num_colons == 2) {
-          // Allocate enough memory to hold the sub-string.
-          transport_code =
-              malloc(sizeof(char) * ((i - start_segment_index) + 1));
-          *(transport_code + (i - start_segment_index)) = '\0';
-          // Copy the sub-string to the freshly allocated memory area.
-          strncpy(transport_code, start_segment, (i - start_segment_index));
-
-          // Set the start of the next segment to directly after the colon.
-          start_segment_index = i + 1;
-          start_segment = connection_string + start_segment_index;
-
-          // The transport code is always followed by "://". So check if this 
is
-          // the case. If it is, switch to question-mark searching mode.
-          if ((*start_segment != '/') || (*(start_segment + 1) != '/')) {
-            return INVALID_CONNECTION_STRING;
-          }
-          mode = SEARCHING_FOR_QUESTION_MARKS;
-
-          // Bump the start of the segment to after the double slashes.
-          start_segment_index += 2;
-          start_segment += 2;
-        } else {
-          return INVALID_CONNECTION_STRING;
-        }
-      }
-    }
-    // If we're in the second part, look for question marks.
-    else {
-      // The question-mark separates the transport connect information from the
-      // parameters.
-      if (*(connection_string + i) == '?') {
-        num_question_marks++;
-        // Only one question-mark is allowed in connection strings.
-        if (num_question_marks > 1) {
-          return INVALID_CONNECTION_STRING;
-        }
 
-        // Allocate enough memory to hold the sub-string.
-        transport_connect_information =
-            malloc(sizeof(char) * ((i - start_segment_index) + 1));
-        *(transport_connect_information + (i - start_segment_index)) = '\0';
-        // Copy the sub-string to the freshly allocated memory area.
-        strncpy(transport_connect_information, start_segment,
-                (i - start_segment_index));
-
-        // Set the start of the next segment to directly after the
-        // question-mark.
-        start_segment_index = i + 1;
-        start_segment = connection_string + start_segment_index;
-      }
-      // This is the last character ... finish up the last loose end.
-      if (i == strlen(connection_string)) {
-        // If no question-mark has been encountered, this connection string
-        // doesn't have one and the remaining part is simply the transport
-        // connect information.
-        if (num_question_marks == 0) {
-          transport_connect_information =
-              malloc(sizeof(char) * ((i - start_segment_index)) + 2);
-          *(transport_connect_information + (i - start_segment_index) + 1) = 
'\0';
-          strncpy(transport_connect_information, start_segment,
-                  (i - start_segment_index) + 1);
-        }
-        // I a question-mark was found, this is the parameters section.
-        else {
-          parameters = malloc(sizeof(char) * (i - start_segment_index) + 2);
-          *(parameters + (i - start_segment_index) + 1) = '\0';
-          strncpy(parameters, start_segment, (i - start_segment_index) + 1);
-        }
-      }
-    }
-  }
-  if (num_colons == 0) {
-    return INVALID_CONNECTION_STRING;
-  }
+  // Make a local copy so not to mess up oringal argument, needed later?
+  strcpy(connection_to_tokenize, connection_string);
 
-  // Initialize a new connection data-structure with the parsed information.
-  plc4c_connection *new_connection = malloc(sizeof(plc4c_connection));
+  // Inisalise the connection and set full connection string argument
+  new_connection = malloc(sizeof(plc4c_connection));
   plc4c_connection_initialize(new_connection);
   plc4c_connection_set_connection_string(new_connection, connection_string);
-  plc4c_connection_set_protocol_code(new_connection, protocol_code);
-  plc4c_connection_set_transport_code(new_connection, transport_code);
-  plc4c_connection_set_transport_connect_information(
-      new_connection, transport_connect_information);
-  plc4c_connection_set_parameters(new_connection, parameters);
 
+  // Get the protocol code (1st item, ':' delimited, required)
+  connection_token = strtok_r(connection_to_tokenize, ":", 
&connection_string_pos);
+  plc4c_connection_set_protocol_code(new_connection, connection_token);
+
+  // As protocol code required, check we found a ':'
+  if (strlen(connection_string_pos) == 0) 
+    return INVALID_CONNECTION_STRING;
+
+  // Check if protocol is followed by '//', if by just one '/' return error
+  if (strncmp(connection_string_pos, "//", 2) == 0) {
+    plc4c_connection_set_transport_code(new_connection, NULL);
+    mode = SEARCHING_FOR_QUESTION_MARKS;
+  } else if (strncmp(connection_string_pos, "/", 1) == 0) {
+    return INVALID_CONNECTION_STRING;
+  }
+
+  // Get the transport code (2nd item, ':' delimited, optional)
+  if (mode == SEARCHING_FOR_COLONS) {
+    connection_token = strtok_r(connection_string_pos, ":", 
&connection_string_pos);
+    plc4c_connection_set_transport_code(new_connection, connection_token);
+    if (strncmp(connection_string_pos, "//", 2)) 
+      return INVALID_CONNECTION_STRING;
+  }
+
+  // Skip over the '//' we have now asserted MUST exist, and search for
+  // connection parameters (last item, '?' delimited, optional).
+  connection_string_pos += 2;

Review comment:
       again NULL checks




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to