From 81cd38751ec421627c06029bec801df1ebb78b40 Mon Sep 17 00:00:00 2001 From: Lawrence Lee Date: Mon, 8 Apr 2024 20:41:18 +0000 Subject: [PATCH] [ssg]: Use C++ strings for text handling Signed-off-by: Lawrence Lee --- src/systemd-sonic-generator/Makefile | 16 +- src/systemd-sonic-generator/ssg-test.cc | 11 +- ...enerator.c => systemd-sonic-generator.cpp} | 429 ++++++++---------- .../systemd-sonic-generator.h | 18 +- 4 files changed, 202 insertions(+), 272 deletions(-) rename src/systemd-sonic-generator/{systemd-sonic-generator.c => systemd-sonic-generator.cpp} (73%) diff --git a/src/systemd-sonic-generator/Makefile b/src/systemd-sonic-generator/Makefile index ecfc19e0b26a..c4c01f66f470 100644 --- a/src/systemd-sonic-generator/Makefile +++ b/src/systemd-sonic-generator/Makefile @@ -2,15 +2,15 @@ CC=gcc CFLAGS += -std=gnu99 -D_GNU_SOURCE CXX=g++ -CXXFLAGS += -std=c++11 -D_GNU_SOURCE -LDFLAGS += -lpthread -lboost_filesystem -lboost_system -lgtest -ljson-c +CXXFLAGS += -std=c++11 -D_GNU_SOURCE -I ./ +LDFLAGS += -l stdc++ -lpthread -lboost_filesystem -lboost_system -lgtest -ljson-c BINARY = systemd-sonic-generator -$(BINARY): systemd-sonic-generator.c +$(BINARY): systemd-sonic-generator.cpp rm -f ./systemd-sonic-generator - $(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS) + $(CXX) $(CXXFLAGS) -o $@ $^ $(LDFLAGS) install: $(BINARY) mkdir -p $(DESTDIR) @@ -24,10 +24,12 @@ test: ssg_test ./ssg_test ssg_test: ssg-test.cc systemd-sonic-generator.o - $(CXX) $(CXXFLAGS) -o $@ $^ $(LDFLAGS) + $(CXX) $(CXXFLAGS) -ggdb -o $@ $^ $(LDFLAGS) + +systemd-sonic-generator.o: systemd-sonic-generator.cpp + $(CXX) $(CXXFLAGS) -ggdb -D_SSG_UNITTEST -o $@ -c $^ -systemd-sonic-generator.o: systemd-sonic-generator.c - $(CC) $(CFLAGS) -D_SSG_UNITTEST -o $@ -c $^ +all: $(BINARY) test clean: rm -f ./systemd-sonic-generator diff --git a/src/systemd-sonic-generator/ssg-test.cc b/src/systemd-sonic-generator/ssg-test.cc index bcc661a9df2a..2462c0635ad9 100644 --- a/src/systemd-sonic-generator/ssg-test.cc +++ b/src/systemd-sonic-generator/ssg-test.cc @@ -210,8 +210,7 @@ class SsgMainTest : public SsgFunctionTest { /* Find a string in a file */ bool find_string_in_file(std::string str, - std::string file_name, - int num_asics) { + std::string file_name) { bool found = false; std::string line; @@ -243,7 +242,7 @@ class SsgMainTest : public SsgFunctionTest { /* Run once for single instance */ finished = true; } - EXPECT_EQ(find_string_in_file(str_t, target, num_asics), + EXPECT_EQ(find_string_in_file(str_t, target), expected_result) << "Error validating " + str_t + " in " + target; } @@ -600,9 +599,8 @@ TEST_F(SsgFunctionTest, insert_instance_number) { char input[] = "test@.service"; for (int i = 0; i <= 100; ++i) { std::string out = "test@" + std::to_string(i) + ".service"; - char* ret = insert_instance_number(input, i, ""); - ASSERT_NE(ret, nullptr); - EXPECT_STREQ(ret, out.c_str()); + std::string ret = insert_instance_number(input, i, ""); + EXPECT_EQ(ret, out); } } @@ -670,7 +668,6 @@ TEST_F(SsgFunctionTest, get_unit_files) { /* TEST ssg_main() argv error */ TEST_F(SsgMainTest, ssg_main_argv) { - FILE* fp; std::vector argv_; std::vector arguments = { "ssg_main", diff --git a/src/systemd-sonic-generator/systemd-sonic-generator.c b/src/systemd-sonic-generator/systemd-sonic-generator.cpp similarity index 73% rename from src/systemd-sonic-generator/systemd-sonic-generator.c rename to src/systemd-sonic-generator/systemd-sonic-generator.cpp index f5cac8e942f2..d1b61a325d3c 100644 --- a/src/systemd-sonic-generator/systemd-sonic-generator.c +++ b/src/systemd-sonic-generator/systemd-sonic-generator.cpp @@ -1,14 +1,16 @@ -#include #include #include #include #include #include -#include +// #include #include #include #include #include +#include +#include +#include #define MAX_NUM_TARGETS 48 #define MAX_NUM_INSTALL_LINES 48 @@ -69,7 +71,7 @@ static int num_multi_inst; static bool smart_switch_npu; static bool smart_switch_dpu; static bool smart_switch; -static size_t num_dpus; +static int num_dpus; static char* platform = NULL; static struct json_object *platform_info = NULL; @@ -122,11 +124,23 @@ void strip_trailing_newline(char* str) { Strips trailing newline from a string if it exists ***/ + if (str == NULL) { + return; + } size_t l = strlen(str); if (l > 0 && str[l-1] == '\n') str[l-1] = '\0'; } +void strip_trailing_newline(std::string& str) { + /*** + Strips trailing newline from a string if it exists + ***/ + if (!str.empty() && str.back() == '\n') { + str.pop_back(); + } +} + /** * Checks if the given path is "/dev/null". @@ -144,7 +158,7 @@ static bool is_devnull(const char* path) } -static int get_target_lines(char* unit_file, char* target_lines[]) { +static int get_target_lines(const char* unit_file, char* target_lines[]) { /*** Gets installation information for a given unit file @@ -191,26 +205,32 @@ static int get_target_lines(char* unit_file, char* target_lines[]) { return num_target_lines; } -static bool is_multi_instance_service(char *service_name){ - int i; - for(i=0; i < num_multi_inst; i++){ - /* - * The service name may contain @.service or .service. Remove these - * postfixes and extract service name. Compare service name for absolute - * match in multi_instance_services[]. - * This is to prevent services like database-chassis and systemd-timesyncd marked - * as multi instance services as they contain strings 'database' and 'syncd' respectively - * which are multi instance services in multi_instance_services[]. - */ - char *saveptr; - char *token = strtok_r(service_name, "@", &saveptr); - if (token) { - if (strstr(token, ".service") != NULL) { - /* If we are here, service_name did not have '@' delimiter but contains '.service' */ - token = strtok_r(service_name, ".", &saveptr); +static bool is_multi_instance_service(std::string service_file, std::unordered_set service_list=std::unordered_set()){ + /* + * The service name may contain @.service or .service. Remove these + * postfixes and extract service name. Compare service name for absolute + * match in multi_instance_services[]. + * This is to prevent services like database-chassis and systemd-timesyncd marked + * as multi instance services as they contain strings 'database' and 'syncd' respectively + * which are multi instance services in multi_instance_services[]. + */ + std::string delimiter; + if (service_file.find("@") != std::string::npos) { + delimiter = "@"; + } else { + delimiter = "."; + } + std::string service_name = service_file.substr(0, service_file.find(delimiter)); + + if (service_list.empty()) { + for(int i=0; i < num_multi_inst; i++){ + + if (service_name == multi_instance_services[i]) { + return true; } } - if (strncmp(service_name, multi_instance_services[i], strlen(service_name)) == 0) { + } else { + if (service_list.count(service_name) > 0) { return true; } } @@ -225,97 +245,59 @@ static bool is_multi_instance_service(char *service_name){ * @param service_name The name of the service to check. * @return true if the service is a multi-instance service for DPU, false otherwise. */ -static bool is_multi_instance_service_for_dpu(const char *service_name) { +static bool is_multi_instance_service_for_dpu(const std::string& service_name) { if (!smart_switch_npu) { return false; } - const static char* multi_instance_services_for_dpu[] = {"database"}; - char *tmp_service_name = strdup(service_name); - if (tmp_service_name == NULL) { - fprintf(stderr, "Error: Failed to allocate memory for tmp_service_name\n"); - exit(EXIT_FAILURE); - } - - for (size_t i = 0; i < sizeof(multi_instance_services_for_dpu) / - sizeof(multi_instance_services_for_dpu[0]); - i++) { - char* saveptr; - char* token = strtok_r(tmp_service_name, "@", &saveptr); - if (token) { - if (strstr(token, ".service") != NULL) { - /* If we are here, service_name did not have '@' delimiter but - * contains '.service' */ - token = strtok_r(tmp_service_name, ".", &saveptr); - } - } - if (strcmp(tmp_service_name, multi_instance_services_for_dpu[i]) == 0) { - free(tmp_service_name); - return true; - } - } - - free(tmp_service_name); - return false; + std::unordered_set multi_instance_services_for_dpu = {"database"}; + return is_multi_instance_service(service_name, multi_instance_services_for_dpu); } -static int get_install_targets_from_line(char* target_string, char* install_type, char* targets[], int existing_targets) { +static int get_install_targets_from_line(std::string target_string, std::string install_type, char* targets[], int existing_targets) { /*** Helper fuction for get_install_targets Given a space delimited string of target directories and a suffix, puts each target directory plus the suffix into the targets array ***/ - char* token; - char* target; - char* saveptr; - char final_target[PATH_MAX]; + std::string target; int num_targets = 0; - assert(target_string); - assert(install_type); - - while ((token = strtok_r(target_string, " ", &target_string))) { - if (num_targets + existing_targets >= MAX_NUM_TARGETS) { - fputs("Number of targets found exceeds MAX_NUM_TARGETS\n", stderr); - fputs("Additional targets will be ignored \n", stderr); - return num_targets; - } - - target = strdup(token); - strip_trailing_newline(target); + if (target_string.empty() || install_type.empty()) { + fprintf(stderr, "Invalid target string or install type\n"); + exit(EXIT_FAILURE); + } - if (strstr(target, "%") != NULL) { - char* prefix = strtok_r(target, ".", &saveptr); - char* suffix = strtok_r(NULL, ".", &saveptr); - int prefix_len = strlen(prefix); + std::stringstream ss(target_string); - strncpy(final_target, prefix, prefix_len - 2); - final_target[prefix_len - 2] = '\0'; - strcat(final_target, "."); - strcat(final_target, suffix); + while (ss >> target) { + if (num_targets + existing_targets >= MAX_NUM_TARGETS) { + fprintf(stderr, "Number of targets exceeds MAX_NUM_TARGETS\n"); + fputs("Additional targets will be ignored\n", stderr); + break; } - else { - strcpy(final_target, target); + // handle install targets using the '%i' systemd specifier + if (target.find("%") != std::string::npos) { + target = target.substr(0, target.find("%")) + target.substr(target.find(".")); } - strcat(final_target, install_type); - - free(target); - - targets[num_targets + existing_targets] = strdup(final_target); + strip_trailing_newline(target); + target += install_type; + targets[num_targets + existing_targets] = (char*) calloc(target.length() + 1, sizeof(char)); + snprintf(targets[num_targets + existing_targets], PATH_MAX, "%s", target.c_str()); num_targets++; } return num_targets; } -static void replace_multi_inst_dep(char *src) { +static void replace_multi_inst_dep(const char *src) { FILE *fp_src; FILE *fp_tmp; char buf[MAX_BUF_SIZE]; char* line = NULL; int i; - ssize_t len; + size_t len; char *token; char *word; char *line_copy; @@ -395,14 +377,14 @@ static void replace_multi_inst_dep(char *src) { rename(tmp_file_path, src); } -int get_install_targets(char* unit_file, char* targets[]) { +int get_install_targets(std::string unit_file, char* targets[]) { /*** Returns install targets for a unit file Parses the information in the [Install] section of a given unit file to determine which directories to install the unit in ***/ - char file_path[PATH_MAX]; + std::string file_path; char *target_lines[MAX_NUM_INSTALL_LINES]; int num_target_lines; int num_targets; @@ -410,26 +392,21 @@ int get_install_targets(char* unit_file, char* targets[]) { char* token; char* line = NULL; bool first; - char* target_suffix = NULL; - char *instance_name; - char *dot_ptr; + std::string target_suffix; + std::string instance_name; - strcpy(file_path, get_unit_file_prefix()); - strcat(file_path, unit_file); + file_path = get_unit_file_prefix() + unit_file; - instance_name = strdup(unit_file); - dot_ptr = strchr(instance_name, '.'); - *dot_ptr = '\0'; + instance_name = unit_file.substr(0, unit_file.find('.')); if(((num_asics > 1) && (!is_multi_instance_service(instance_name))) || ((num_dpus > 0) && (!is_multi_instance_service_for_dpu(instance_name)))) { - replace_multi_inst_dep(file_path); + replace_multi_inst_dep(file_path.c_str()); } - free(instance_name); - num_target_lines = get_target_lines(file_path, target_lines); + num_target_lines = get_target_lines(file_path.c_str(), target_lines); if (num_target_lines < 0) { - fprintf(stderr, "Error parsing targets for %s\n", unit_file); + fprintf(stderr, "Error parsing targets for %s\n", unit_file.c_str()); return -1; } @@ -484,7 +461,7 @@ int get_unit_files(char* unit_files[]) { int num_unit_files = 0; num_multi_inst = 0; - multi_instance_services = calloc(MAX_NUM_UNITS, sizeof(char *)); + multi_instance_services = (char**) calloc(MAX_NUM_UNITS, sizeof(char *)); while ((read = getline(&line, &len, fp)) != -1) { if (num_unit_files >= MAX_NUM_UNITS) { @@ -496,8 +473,8 @@ int get_unit_files(char* unit_files[]) { /* Get the multi-instance services */ pos = strchr(line, '@'); if (pos != NULL) { - multi_instance_services[num_multi_inst] = calloc(strlen(line), sizeof(char)); - strncpy(multi_instance_services[num_multi_inst], line, pos-line); + multi_instance_services[num_multi_inst] = (char*) calloc(pos-line+1, sizeof(char)); + snprintf(multi_instance_services[num_multi_inst], pos-line+1, "%s", line); num_multi_inst++; } @@ -525,111 +502,87 @@ int get_unit_files(char* unit_files[]) { } -char* insert_instance_number(char* unit_file, int instance, const char *instance_prefix) { +std::string insert_instance_number(const std::string& unit_file, int instance, const std::string& instance_prefix) { /*** Adds an instance number to a systemd template name E.g. given unit_file='example@.service', instance=3, returns a pointer to 'example@3.service' ***/ - char* instance_name; - int ret; - int prefix_len; - const char *suffix = strchr(unit_file, '@'); - if (!suffix) { - fprintf(stderr, "Invalid unit file %s for instance %d\n", unit_file, instance); - return NULL; - } - - if (instance_prefix == NULL) { - instance_prefix = ""; - } - - /*** - suffix is "@.service", set suffix=".service" - prefix_len is length of "example@" - ***/ - prefix_len = ++suffix - unit_file; - ret = asprintf(&instance_name, "%.*s%s%d%s", prefix_len, unit_file, instance_prefix, instance, suffix); - if (ret == -1) { - fprintf(stderr, "Error creating instance %d of %s\n", instance, unit_file); - return NULL; + size_t at_pos = unit_file.find("@"); + if (at_pos == std::string::npos) { + fprintf(stderr, "Invalid unit file %s for instance %d\n", unit_file.c_str(), instance); + return ""; } - return instance_name; + return unit_file.substr(0, at_pos + 1) + instance_prefix + std::to_string(instance) + unit_file.substr(at_pos + 1); } -static int create_symlink(char* unit, char* target, char* install_dir, int instance, const char *instance_prefix) { +static int create_symlink(const std::string& unit, const std::string& target, const std::string& install_dir, int instance, const std::string& instance_prefix) { struct stat st; - char src_path[PATH_MAX]; - char dest_path[PATH_MAX]; - char final_install_dir[PATH_MAX]; - char* unit_instance; + std::string src_path; + std::string dest_path; + std::string final_install_dir; + std::string unit_instance; int r; - strcpy(src_path, get_unit_file_prefix()); - strcat(src_path, unit); + src_path = get_unit_file_prefix() + unit; if (instance < 0) { - unit_instance = strdup(unit); + unit_instance = unit; } else { unit_instance = insert_instance_number(unit, instance, instance_prefix); } - strcpy(final_install_dir, install_dir); - strcat(final_install_dir, target); - strcpy(dest_path, final_install_dir); - strcat(dest_path, "/"); - strcat(dest_path, unit_instance); + final_install_dir = install_dir + std::string(target); + dest_path = final_install_dir + "/" + unit_instance; - free(unit_instance); - - if (stat(final_install_dir, &st) == -1) { + if (stat(final_install_dir.c_str(), &st) == -1) { // If doesn't exist, create - r = mkdir(final_install_dir, 0755); + r = mkdir(final_install_dir.c_str(), 0755); if (r == -1) { - fprintf(stderr, "Unable to create target directory %s\n", final_install_dir); + fprintf(stderr, "Unable to create target directory %s\n", final_install_dir.c_str()); return -1; } } else if (S_ISREG(st.st_mode)) { // If is regular file, remove and create - r = remove(final_install_dir); + r = remove(final_install_dir.c_str()); if (r == -1) { - fprintf(stderr, "Unable to remove file with same name as target directory %s\n", final_install_dir); + fprintf(stderr, "Unable to remove file with same name as target directory %s\n", final_install_dir.c_str()); return -1; } - r = mkdir(final_install_dir, 0755); + r = mkdir(final_install_dir.c_str(), 0755); if (r == -1) { - fprintf(stderr, "Unable to create target directory %s\n", final_install_dir); + fprintf(stderr, "Unable to create target directory %s\n", final_install_dir.c_str()); return -1; } } else if (S_ISDIR(st.st_mode)) { // If directory, verify correct permissions - r = chmod(final_install_dir, 0755); + r = chmod(final_install_dir.c_str(), 0755); if (r == -1) { - fprintf(stderr, "Unable to change permissions of existing target directory %s\n", final_install_dir); + fprintf(stderr, "Unable to change permissions of existing target directory %s\n", final_install_dir.c_str()); return -1; } } - if (is_devnull(dest_path)) { - if (remove(dest_path) != 0) { - fprintf(stderr, "Unable to remove existing symlink %s\n", dest_path); + if (is_devnull(dest_path.c_str())) { + if (remove(dest_path.c_str()) != 0) { + fprintf(stderr, "Unable to remove existing symlink %s\n", dest_path.c_str()); return -1; } } - r = symlink(src_path, dest_path); + r = symlink(src_path.c_str(), dest_path.c_str()); if (r < 0) { if (errno == EEXIST) return 0; - fprintf(stderr, "Error creating symlink %s from source %s\n", dest_path, src_path); + fprintf(stderr, "Error creating symlink %s from source %s\n", dest_path.c_str(), src_path.c_str()); return -1; } @@ -638,7 +591,7 @@ static int create_symlink(char* unit, char* target, char* install_dir, int insta } -static int install_unit_file(char* unit_file, char* target, char* install_dir) { +static int install_unit_file(std::string unit_file, std::string target, std::string install_dir) { /*** Creates a symlink for a unit file installation @@ -649,34 +602,30 @@ static int install_unit_file(char* unit_file, char* target, char* install_dir) { If a multi ASIC platform is detected, enables multi-instance services as well ***/ - char* target_instance; - char* prefix; - char* suffix; + std::string target_instance; int r; - assert(unit_file); - assert(target); - + if (unit_file.empty() || target.empty() || install_dir.empty()){ + fprintf(stderr, "Invalid unit file, target or install directory\n"); + exit(EXIT_FAILURE); + } - if ((num_asics > 1) && strstr(unit_file, "@") != NULL) { + if ((num_asics > 1) && unit_file.find("@") != std::string::npos) { for (int i = 0; i < num_asics; i++) { - if (strstr(target, "@") != NULL) { + if (target.find("@") != std::string::npos) { target_instance = insert_instance_number(target, i, ""); } else { - target_instance = strdup(target); + target_instance = target; } r = create_symlink(unit_file, target_instance, install_dir, i, ""); if (r < 0) - fprintf(stderr, "Error installing %s for target %s\n", unit_file, target_instance); - - free(target_instance); - + fprintf(stderr, "Error installing %s for target %s\n", unit_file.c_str(), target_instance.c_str()); } - } else if (num_dpus > 0 && strstr(unit_file, "@") != NULL) { + } else if (num_dpus > 0 && unit_file.find("@") != std::string::npos) { // If multi-instance service for DPU // Install each DPU units to the host main instance only, // E.g. install database@dpu0.service, database@dpu1.service to multi-user.target.wants @@ -684,12 +633,12 @@ static int install_unit_file(char* unit_file, char* target, char* install_dir) { for (int i = 0; i < num_dpus; i++) { r = create_symlink(unit_file, target, install_dir, i, DPU_PREFIX); if (r < 0) - fprintf(stderr, "Error installing %s for target %s\n", unit_file, target); + fprintf(stderr, "Error installing %s for target %s\n", unit_file.c_str(), target.c_str()); } } else { r = create_symlink(unit_file, target, install_dir, -1, ""); if (r < 0) - fprintf(stderr, "Error installing %s for target %s\n", unit_file, target); + fprintf(stderr, "Error installing %s for target %s\n", unit_file.c_str(), target.c_str()); } return 0; @@ -714,7 +663,6 @@ const char* get_platform() { FILE* fp; char* line = NULL; - char* token; char* saveptr; char *tmp_platform = NULL; static char platform_buffer[MAX_PLATFORM_NAME_LEN + 1]; @@ -730,7 +678,7 @@ const char* get_platform() { while ((nread = getline(&line, &len, fp)) != -1) { if ((strstr(line, "onie_platform") != NULL) || (strstr(line, "aboot_platform") != NULL)) { - token = strtok_r(line, "=", &saveptr); + strtok_r(line, "=", &saveptr); tmp_platform = strtok_r(NULL, "=", &saveptr); strip_trailing_newline(tmp_platform); break; @@ -742,7 +690,7 @@ const char* get_platform() { free(line); return NULL; } - strncpy(platform_buffer, tmp_platform, sizeof(platform_buffer) - 1); + snprintf(platform_buffer, sizeof(platform_buffer), "%s", tmp_platform); fclose(fp); free(line); @@ -757,7 +705,6 @@ int get_num_of_asic() { ***/ FILE *fp; char *line = NULL; - char* token; const char* platform = NULL; char* saveptr; size_t len = 0; @@ -774,7 +721,7 @@ int get_num_of_asic() { if (fp != NULL) { while ((nread = getline(&line, &len, fp)) != -1) { if (strstr(line, "NUM_ASIC") != NULL) { - token = strtok_r(line, "=", &saveptr); + strtok_r(line, "=", &saveptr); str_num_asic = strtok_r(NULL, "=", &saveptr); strip_trailing_newline(str_num_asic); if (str_num_asic != NULL){ @@ -834,7 +781,7 @@ const struct json_object* get_platform_info() { fclose(fp); exit(EXIT_FAILURE); } - char *platform_json = malloc(fsize + 1); + char *platform_json = (char*) malloc(fsize + 1); if (platform_json == NULL) { fprintf(stdout, "Failed to allocate memory for %s\n", platform_file_path); fclose(fp); @@ -925,58 +872,54 @@ static int get_num_of_dpu() { * @param unit_name The name of the network unit to install. * @return 0 if the network unit is installed successfully, or -1 if an error occurs. */ -static int install_network_unit(const char* unit_name) { - assert(unit_name); +static int install_network_unit(std::string unit_name) { + if (unit_name.empty()) { + fprintf(stderr, "Invalid network unit\n"); + exit(EXIT_FAILURE); + } - const char* unit_type = strrchr(unit_name, '.'); - if (unit_type == NULL) { - fprintf(stderr, "Invalid network unit %s\n", unit_name); + std::string unit_type = unit_name.substr(unit_name.find(".") + 1); + if (unit_type.empty()) { + fprintf(stderr, "Invalid network unit %s\n", unit_name.c_str()); return -1; } - unit_type++; - char install_path[PATH_MAX] = {0}; - char original_path[PATH_MAX] = {0}; - const char* subdir; - if (strcmp(unit_type, "netdev") == 0 || strcmp(unit_type, "network") == 0) { - subdir = "/network/"; - } else { - fprintf(stderr, "Invalid network unit %s\n", unit_type); + std::string install_path; + std::string original_path; + std::string subdir = "/network/"; + if (unit_type != "netdev" && unit_type != "network") { + fprintf(stderr, "Invalid network unit %s\n", unit_type.c_str()); return -1; } - strcpy(install_path, get_etc_systemd()); - strcat(install_path, subdir); - strcat(install_path, unit_name); - strcpy(original_path, get_lib_systemd()); - strcat(original_path, subdir); - strcat(original_path, unit_name); + install_path = get_etc_systemd() + subdir + unit_name; + original_path = get_lib_systemd() + subdir + unit_name; struct stat st; - if (stat((const char *)install_path, &st) == 0) { + if (stat(install_path.c_str(), &st) == 0) { // If the file already exists, remove it if (S_ISDIR(st.st_mode)) { - fprintf(stderr, "Error: %s is a directory\n", install_path); + fprintf(stderr, "Error: %s is a directory\n", install_path.c_str()); return -1; } - if (remove(install_path) != 0) { - fprintf(stderr, "Error removing existing file %s\n", install_path); + if (remove(install_path.c_str()) != 0) { + fprintf(stderr, "Error removing existing file %s\n", install_path.c_str()); return -1; } } - if (is_devnull(install_path)) { - if (remove(install_path) != 0) { - fprintf(stderr, "Unable to remove existing symlink %s\n", install_path); + if (is_devnull(install_path.c_str())) { + if (remove(install_path.c_str()) != 0) { + fprintf(stderr, "Unable to remove existing symlink %s\n", install_path.c_str()); return -1; } } - if (symlink(original_path, install_path) != 0) { + if (symlink(original_path.c_str(), install_path.c_str()) != 0) { if (errno == EEXIST) return 0; - fprintf(stderr, "Error creating symlink %s -> %s (%s)\n", install_path, original_path, strerror(errno)); + fprintf(stderr, "Error creating symlink %s -> %s (%s)\n", install_path.c_str(), original_path.c_str(), strerror(errno)); return -1; } @@ -994,39 +937,34 @@ static int render_network_service_for_smart_switch() { return 0; } - char buffer_instruction[MAX_BUF_SIZE] = {0}; - strcpy(buffer_instruction, "\nBefore="); - for (size_t i = 0; i < num_dpus; i++) { - char *unit; - asprintf(&unit, "database@dpu%ld.service", i); - strcat(buffer_instruction, unit); - free(unit); + std::stringstream ss; + ss << "\nBefore="; + for (int i = 0; i < num_dpus; i++) { + ss << "database@dpu" << i << ".service"; if (i != num_dpus - 1) { - strcat(buffer_instruction, " "); + ss << " "; } } + std::string buffer_instruction = ss.str(); + std::string unit_path = std::string(get_unit_file_prefix()) + "/midplane-network-npu.service"; - char unit_path[PATH_MAX] = { 0 }; - strcpy(unit_path, get_unit_file_prefix()); - strcat(unit_path, "/midplane-network-npu.service"); - - FILE *fp = fopen(unit_path, "r"); + FILE *fp = fopen(unit_path.c_str(), "r"); if (fp == NULL) { - fprintf(stderr, "Failed to open %s\n", unit_path); + fprintf(stderr, "Failed to open %s\n", unit_path.c_str()); return -1; } fseek(fp, 0, SEEK_END); size_t file_size = ftell(fp); fseek(fp, 0, SEEK_SET); - size_t len = file_size + strlen(buffer_instruction) + 1; - char *unit_content = malloc(len); + size_t len = file_size + buffer_instruction.length() + 1; + char *unit_content = (char*) malloc(len); if (unit_content == NULL) { - fprintf(stderr, "Failed to allocate memory for %s\n", unit_path); + fprintf(stderr, "Failed to allocate memory for %s\n", unit_path.c_str()); fclose(fp); exit(EXIT_FAILURE); } if (fread(unit_content, file_size, 1, fp) != 1) { - fprintf(stderr, "Failed to read %s\n", unit_path); + fprintf(stderr, "Failed to read %s\n", unit_path.c_str()); free(unit_content); fclose(fp); exit(EXIT_FAILURE); @@ -1037,11 +975,11 @@ static int render_network_service_for_smart_switch() { char *insert_point = strstr(unit_content, "[Unit]"); insert_point += strlen("[Unit]"); // Move the rest of the file to make room for the Before instruction - memmove(insert_point + strlen(buffer_instruction), insert_point, file_size - (insert_point - unit_content)); + memmove(insert_point + buffer_instruction.length(), insert_point, file_size - (insert_point - unit_content)); // Insert the Before instruction - memcpy(insert_point, buffer_instruction, strlen(buffer_instruction)); + memcpy(insert_point, buffer_instruction.c_str(), buffer_instruction.length()); // Remove original Before instruction - insert_point += strlen(buffer_instruction); + insert_point += buffer_instruction.length(); char *before_start = strstr(insert_point, "Before="); while (before_start != NULL) { char *before_end = strchr(before_start, '\n'); @@ -1060,14 +998,14 @@ static int render_network_service_for_smart_switch() { before_start = strstr(before_start, "Before="); } // Write the modified unit file - fp = fopen(unit_path, "w"); + fp = fopen(unit_path.c_str(), "w"); if (fp == NULL) { - fprintf(stderr, "Failed to open %s\n", unit_path); + fprintf(stderr, "Failed to open %s\n", unit_path.c_str()); free(unit_content); exit(EXIT_FAILURE); } if (fwrite(unit_content, strlen(unit_content), 1, fp) != 1) { - fprintf(stderr, "Failed to write %s\n", unit_path); + fprintf(stderr, "Failed to write %s\n", unit_path.c_str()); free(unit_content); fclose(fp); exit(EXIT_FAILURE); @@ -1118,15 +1056,13 @@ static int install_network_service_for_smart_switch() { int ssg_main(int argc, char **argv) { char* unit_files[MAX_NUM_UNITS]; - char install_dir[PATH_MAX]; + std::string install_dir; char* targets[MAX_NUM_TARGETS]; - char* unit_instance; - char* prefix; - char* suffix; - char* saveptr; + std::string unit_instance; + std::string prefix; + std::string suffix; int num_unit_files; int num_targets; - int r; #ifdef _SSG_UNITTEST clean_up_cache(); @@ -1143,8 +1079,7 @@ int ssg_main(int argc, char **argv) { smart_switch = smart_switch_npu || smart_switch_dpu; num_dpus = get_num_of_dpu(); - strcpy(install_dir, argv[1]); - strcat(install_dir, "/"); + install_dir = std::string(argv[1]) + "/"; num_unit_files = get_unit_files(unit_files); // Install and render midplane network service for smart switch @@ -1159,36 +1094,30 @@ int ssg_main(int argc, char **argv) { // For each unit file, get the installation targets and install the unit for (int i = 0; i < num_unit_files; i++) { - unit_instance = strdup(unit_files[i]); + unit_instance = unit_files[i]; if ((num_asics == 1 && !is_multi_instance_service_for_dpu(unit_instance)) && - strstr(unit_instance, "@") != NULL) { - prefix = strdup(strtok_r(unit_instance, "@", &saveptr)); - suffix = strdup(strtok_r(NULL, "@", &saveptr)); - - strcpy(unit_instance, prefix); - strcat(unit_instance, suffix); + unit_instance.find("@") != std::string::npos) { + prefix = unit_instance.substr(0, unit_instance.find("@")); + suffix = unit_instance.substr(unit_instance.find("@") + 1); - free(prefix); - free(suffix); + unit_instance = prefix + suffix; } num_targets = get_install_targets(unit_instance, targets); if (num_targets < 0) { - fprintf(stderr, "Error parsing %s\n", unit_instance); - free(unit_instance); + fprintf(stderr, "Error parsing %s\n", unit_instance.c_str()); free(unit_files[i]); continue; } for (int j = 0; j < num_targets; j++) { if (install_unit_file(unit_instance, targets[j], install_dir) != 0) - fprintf(stderr, "Error installing %s to target directory %s\n", unit_instance, targets[j]); + fprintf(stderr, "Error installing %s to target directory %s\n", unit_instance.c_str(), targets[j]); free(targets[j]); } - free(unit_instance); free(unit_files[i]); } diff --git a/src/systemd-sonic-generator/systemd-sonic-generator.h b/src/systemd-sonic-generator/systemd-sonic-generator.h index 8695951fb211..91e2d476c1ac 100644 --- a/src/systemd-sonic-generator/systemd-sonic-generator.h +++ b/src/systemd-sonic-generator/systemd-sonic-generator.h @@ -6,9 +6,11 @@ * Copyright (c) 2021 by Cisco Systems, Inc. *------------------------------------------------------------------ */ -#ifdef __cplusplus -extern "C" { -#endif +// #ifdef __cplusplus +// extern "C" { +// #endif +#include +#include /* expose global vars for testing purpose */ extern const char* UNIT_FILE_PREFIX; @@ -29,11 +31,11 @@ extern const char* get_unit_file_prefix(); extern const char* get_config_file(); extern const char* get_machine_config_file(); extern const char* get_asic_conf_format(); -extern char* insert_instance_number(char* unit_file, int instance, const char *instance_prefix); +extern std::string insert_instance_number(const std::string& unit_file, int instance, const std::string& instance_prefix); extern int ssg_main(int argc, char** argv); extern int get_num_of_asic(); -extern int get_install_targets(char* unit_file, char* targets[]); +extern int get_install_targets(std::string unit_file, char* targets[]); extern int get_unit_files(char* unit_files[]); -#ifdef __cplusplus -} -#endif +// #ifdef __cplusplus +// } +// #endif