[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2 integration.
arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2 integration. URL: https://github.com/apache/nifi-minifi-cpp/pull/674#discussion_r353279748 ## File path: nanofi/ecu/coap_server.c ## @@ -0,0 +1,138 @@ +#include Review comment: Multiple issues here: - License - What is the goal of this file? Are we trying to emulate a C2 server? Some comments would be nice - A server including the header of the agent is a bad smell 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2 integration.
arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2
integration.
URL: https://github.com/apache/nifi-minifi-cpp/pull/674#discussion_r353260326
##
File path: extensions/coap/nanofi/coap_functions.c
##
@@ -105,12 +117,18 @@ int create_endpoint_context(coap_context_t **ctx, const
char *node, const char *
if (*ctx && ep_udp) {
freeaddrinfo(result);
Review comment:
I would prefer just break out of the loop here instead of return, so
resource cleanup can happen at one place.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2 integration.
arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2
integration.
URL: https://github.com/apache/nifi-minifi-cpp/pull/674#discussion_r353269082
##
File path: nanofi/ecu/c2_client.c
##
@@ -0,0 +1,235 @@
+#ifdef __cplusplus
+extern "C" {
+#endif
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+#include "utlist.h"
+#include "uthash.h"
+
+typedef struct config_params {
+char * key; // name of the param
+char * value; // value of the param
+UT_hash_handle hh;
+} config_params;
+
+char ** parse_line(char * line, size_t len) {
+char * tok = strtok(line, " =");
+char ** tokens = (char **)malloc(sizeof(char *) * 2);
+int i = 0;
+while (tok) {
+if (i > 2) break;
+size_t s = strlen(tok);
+char * token = (char *)malloc(sizeof(char) * (s + 1));
+memset(token, 0, (s + 1));
+strcpy(token, tok);
+tokens[i++] = token;
+tok = strtok(NULL, " =\n");
+}
+return tokens;
Review comment:
This function seems to be wrong in multiple ways:
- The parameter called "len" is completely unused.
- It's called parse_line, actually it acts as a string tokeniser, but stop
tokenising at 3 tokens.
- Even though it stops tokenising at 3 tokens (i can be 0, 1 or 2), it only
allocates memory for 2, so it's going to segfault.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2 integration.
arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2
integration.
URL: https://github.com/apache/nifi-minifi-cpp/pull/674#discussion_r353265177
##
File path: nanofi/ecu/c2_client.c
##
@@ -0,0 +1,235 @@
+#ifdef __cplusplus
+extern "C" {
+#endif
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+#include "utlist.h"
+#include "uthash.h"
+
+typedef struct config_params {
+char * key; // name of the param
+char * value; // value of the param
+UT_hash_handle hh;
+} config_params;
+
+char ** parse_line(char * line, size_t len) {
+char * tok = strtok(line, " =");
+char ** tokens = (char **)malloc(sizeof(char *) * 2);
+int i = 0;
+while (tok) {
+if (i > 2) break;
+size_t s = strlen(tok);
+char * token = (char *)malloc(sizeof(char) * (s + 1));
+memset(token, 0, (s + 1));
+strcpy(token, tok);
+tokens[i++] = token;
+tok = strtok(NULL, " =\n");
Review comment:
We have different separator here than in line 24.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2 integration.
arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2
integration.
URL: https://github.com/apache/nifi-minifi-cpp/pull/674#discussion_r353276793
##
File path: nanofi/ecu/c2_client.c
##
@@ -0,0 +1,235 @@
+#ifdef __cplusplus
+extern "C" {
+#endif
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+#include "utlist.h"
+#include "uthash.h"
+
+typedef struct config_params {
+char * key; // name of the param
+char * value; // value of the param
+UT_hash_handle hh;
+} config_params;
+
+char ** parse_line(char * line, size_t len) {
+char * tok = strtok(line, " =");
+char ** tokens = (char **)malloc(sizeof(char *) * 2);
+int i = 0;
+while (tok) {
+if (i > 2) break;
+size_t s = strlen(tok);
+char * token = (char *)malloc(sizeof(char) * (s + 1));
+memset(token, 0, (s + 1));
+strcpy(token, tok);
+tokens[i++] = token;
+tok = strtok(NULL, " =\n");
+}
+return tokens;
+}
+
+struct config_params * read_configuration_file(const char * file_path) {
+if (!file_path) {
+printf("no file path provided\n");
+return NULL;
+}
+
+struct config_params * params = NULL;
+FILE * fp = fopen(file_path, "r");
+char * line = NULL;
+size_t size = 0;
+ssize_t read;
+if (!fp) {
+printf("Cannot open file %s\n", file_path);
+return NULL;
+}
+#ifndef WIN32
+while ((read = getline(, , fp)) > 0) {
+#else
+size = 1024;
+line = (char *)malloc(1024);
+while (fgets(line, size, fp) != NULL) {
+#endif
+char ** tokens = parse_line(line, size);
+struct config_params * el = (struct config_params
*)malloc(sizeof(struct config_params));
+size_t key_len = strlen(tokens[0]);
+size_t value_len = strlen(tokens[1]);
+el->key = (char *)malloc(key_len + 1);
+memset(el->key, 0, key_len + 1);
+strcpy(el->key, tokens[0]);
+
+el->value = (char *)malloc(value_len + 1);
Review comment:
Why do you reallocate, copy and free?
I think the tokens returned should be directly added to the hashmap.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2 integration.
arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2
integration.
URL: https://github.com/apache/nifi-minifi-cpp/pull/674#discussion_r353283926
##
File path: nanofi/src/core/file_utils.c
##
@@ -83,23 +100,62 @@ void remove_directory(const char * dir_path) {
remove_directory(path);
free(path);
}
-
rmdir(dir_path);
+
closedir(d);
}
+#else
+void remove_directory(const char * dir_path) {
+HANDLE hFind;
+WIN32_FIND_DATA fd;
+
+hFind = FindFirstFile(dir_path, );
+if (hFind == INVALID_HANDLE_VALUE) {
+return;
+}
+
+if (fd.dwFileAttributes != FILE_ATTRIBUTE_DIRECTORY) {
+DeleteFile(dir_path);
+FindClose(hFind);
+return;
+}
+
+char * path = concat_path(dir_path, "*");
+HANDLE hFind1;
+if ((hFind1 = FindFirstFile(path, )) != INVALID_HANDLE_VALUE) {
+do {
+char * entry_name = fd.cFileName;
+if (!strcmp(entry_name, ".") || !strcmp(entry_name, ".."))
continue;
+char * entry_path = concat_path(dir_path, entry_name);
+remove_directory(entry_path);
+free(entry_path);
+} while (FindNextFile(hFind1, ));
+}
+RemoveDirectory(dir_path);
+FindClose(hFind);
+FindClose(hFind1);
+free(path);
+}
+#endif
int make_dir(const char * path) {
if (!path) return -1;
errno = 0;
+#ifdef WIN32
+int ret = mkdir(path);
+char path_sep = '\\';
+#else
int ret = mkdir(path,
S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH);
+char path_sep = '/';
Review comment:
Why don't you simpl use "get_separator()"?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2 integration.
arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2
integration.
URL: https://github.com/apache/nifi-minifi-cpp/pull/674#discussion_r353262079
##
File path: extensions/coap/nanofi/coap_server.c
##
@@ -25,6 +25,7 @@ CoapServerContext * const create_server(const char * const
server_hostname, cons
memset(server, 0x00, sizeof(CoapServerContext));
if (create_endpoint_context(>ctx, server_hostname, port)) {
free_server(server);
+return NULL;
Review comment:
Can't comment there at it's not new code, but I think there is also a leak
at line 45 (not freeing endpoint when returning 0)
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2 integration.
arpadboda commented on a change in pull request #674: Minificpp 1007 - ECU C2
integration.
URL: https://github.com/apache/nifi-minifi-cpp/pull/674#discussion_r353257555
##
File path: extensions/coap/DownloadCoapCMakeLists.txt.in
##
@@ -0,0 +1,19 @@
+cmake_minimum_required(VERSION 3.0)
+include(ExternalProject)
+
+find_package(Patch REQUIRED)
+
+set(PATCH_DIR "${CMAKE_CURRENT_SOURCE_DIR}")
+set(LIBCOAP_SRC_DIR "${CMAKE_CURRENT_BINARY_DIR}/libcoap-src")
+
+ExternalProject_Add(
+ libcoap-external
+ GIT_REPOSITORY "https://github.com/obgm/libcoap.git;
+GIT_TAG "d6c25a9a0757af9d13842b7aae9713136e720567"
Review comment:
A version please!
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:
us...@infra.apache.org
With regards,
Apache Git Services
