You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by GitBox <gi...@apache.org> on 2021/03/17 14:17:06 UTC
[GitHub] [plc4x] thomas169 opened a new pull request #229: hello_world_s7 (C) now working
thomas169 opened a new pull request #229:
URL: https://github.com/apache/plc4x/pull/229
hello-world-s7 now working when passed connection string with port eg. `s7:tcp://0.0.0.0:102` I don't think it possible that it was working before... ?
`spi/src/system.c`: plugged mem leaks, removed unneeded mallocs (ie. mallocs in both `spi/src/system.c` and `spi/src/connection.c`, needed only in 1 of these locations), fixed compiler warnings on `hurz` cast and de-globalized `hurz` (maybe globalness was intended?). Rewrote the connection string parser to use tokenisation rather than walking pointer, which is IMO much clearer.
`hello_world_s7.c` beautified some `printf`s and allowed optional cli arguments for setting the test connection string.
`transport_tcp.c` fixed compile warning on `printf` without const format specifier.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [plc4x] ottobackwards commented on a change in pull request #229: hello_world_s7 (C) now working
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #229:
URL: https://github.com/apache/plc4x/pull/229#discussion_r596081254
##########
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);
Review comment:
spelling
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [plc4x] ottobackwards commented on pull request #229: hello_world_s7 (C) now working
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on pull request #229:
URL: https://github.com/apache/plc4x/pull/229#issuecomment-801142344
Thanks for the contribution! Just a couple of questions.
Also, we have to find some way to test the connection string stuff
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [plc4x] ottobackwards commented on a change in pull request #229: hello_world_s7 (C) now working
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [plc4x] hutcheb commented on pull request #229: hello_world_s7 (C) now working
Posted by GitBox <gi...@apache.org>.
hutcheb commented on pull request #229:
URL: https://github.com/apache/plc4x/pull/229#issuecomment-806577808
Hi Thomas,
Just a little confused why this was closed. Closing a PR basically just deletes the PR without having it merged, was that your intention?
Ben
--
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:
users@infra.apache.org
[GitHub] [plc4x] thomas169 commented on a change in pull request #229: hello_world_s7 (C) now working
Posted by GitBox <gi...@apache.org>.
thomas169 commented on a change in pull request #229:
URL: https://github.com/apache/plc4x/pull/229#discussion_r596088873
##########
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:
plc4c_connection_set_transport_code and similar "set" functions check for null hence I didn't, but actually yes if it reutns null subsquency opeations should not be performed as the last example shows
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [plc4x] ottobackwards commented on a change in pull request #229: hello_world_s7 (C) now working
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #229:
URL: https://github.com/apache/plc4x/pull/229#discussion_r596083610
##########
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);
Review comment:
strtok_r can return NULL, this needs to check no?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [plc4x] thomas169 closed pull request #229: hello_world_s7 (C) now working
Posted by GitBox <gi...@apache.org>.
thomas169 closed pull request #229:
URL: https://github.com/apache/plc4x/pull/229
--
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:
users@infra.apache.org
[GitHub] [plc4x] hutcheb commented on pull request #229: hello_world_s7 (C) now working
Posted by GitBox <gi...@apache.org>.
hutcheb commented on pull request #229:
URL: https://github.com/apache/plc4x/pull/229#issuecomment-806717071
https://www.apache.org/licenses/contributor-agreements.html
This explains a bit about what the ICLA/CCLA are and how to submit one.
Welcome to the PLC4X project by the way. :)
--
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:
users@infra.apache.org
[GitHub] [plc4x] ottobackwards commented on a change in pull request #229: hello_world_s7 (C) now working
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #229:
URL: https://github.com/apache/plc4x/pull/229#discussion_r596083100
##########
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)
Review comment:
windows again strtok_s
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [plc4x] hutcheb commented on pull request #229: hello_world_s7 (C) now working
Posted by GitBox <gi...@apache.org>.
hutcheb commented on pull request #229:
URL: https://github.com/apache/plc4x/pull/229#issuecomment-806707519
Yeah that got me at first as well, but I eventually realized that it doesn’t really matter. If you leave anything unintended in a commit you always make another commit to fix it.
GitHub has a decent review section in that it will only show the differences between the code at the last commit and the develop branch. If you add code and then remove it won’t show up in the review.
If you aren’t completely happy with the status of the PR you can always change it to a draft PR so it doesn’t get merged by accident. But using the same PR will keep all the comments and reviews in the one place.
You will also probably need to fill out an agreement with The Apache Foundation (ICLA or CCLA) before something gets merged. I’ll dig up a link that explains it.
--
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:
users@infra.apache.org
[GitHub] [plc4x] ottobackwards commented on a change in pull request #229: hello_world_s7 (C) now working
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #229:
URL: https://github.com/apache/plc4x/pull/229#discussion_r596079902
##########
File path: sandbox/plc4c/examples/hello-world-s7/src/hello_world_s7.c
##########
@@ -21,9 +21,13 @@
#include <plc4c/transport_tcp.h>
#include <stdio.h>
#include <string.h>
Review comment:
Do we need windows support? I think we need some ifdef magic here no?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [plc4x] thomas169 commented on pull request #229: hello_world_s7 (C) now working
Posted by GitBox <gi...@apache.org>.
thomas169 commented on pull request #229:
URL: https://github.com/apache/plc4x/pull/229#issuecomment-806683697
As a git/github noobie I didn't realize that push's to my forked repo automatically went into this PR. Yesterday, I spent the full day getting a write->read loopback test working (fast s7:tcp IO is all I really care about for now) . And commited this, but alas unintentionally to this PR also. So I closed it.
Will make a new PR soon, but the changes are now quite unwieldy (large) for review. Anyway I guess the good news is that I can write a value to a DB and read it back correctly.
--
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:
users@infra.apache.org