diff --git a/src/systemd-sonic-generator/Makefile b/src/systemd-sonic-generator/Makefile index 565fcb803..2e6b769c1 100644 --- a/src/systemd-sonic-generator/Makefile +++ b/src/systemd-sonic-generator/Makefile @@ -1,9 +1,9 @@ CC=gcc CFLAGS=-std=gnu99 -D_GNU_SOURCE -CPP=g++ -CPPFLAGS=-std=c++11 -D_GNU_SOURCE -LFLAGS=-lpthread -lboost_filesystem -lboost_system -lgtest +CXX=g++ +CXXFLAGS += -std=c++11 -D_GNU_SOURCE -I ./ +LDFLAGS += -l stdc++ -lpthread -lboost_filesystem -lboost_system -lgtest BINARY = systemd-sonic-generator MAIN_TARGET = $(BINARY)_1.0.0_$(CONFIGURED_ARCH).deb @@ -13,10 +13,10 @@ $(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% : mv ../$(MAIN_TARGET) $(DEST)/ rm ../$(BINARY)-* ../$(BINARY)_* -$(BINARY): systemd-sonic-generator.c +$(BINARY): systemd-sonic-generator.cpp rm -f ./systemd-sonic-generator - $(CC) $(CFLAGS) -o $@ $^ + $(CXX) $(CXXFLAGS) -o $@ $^ $(LDFLAGS) install: $(BINARY) mkdir -p $(DESTDIR) @@ -30,10 +30,12 @@ test: ssg_test ./ssg_test ssg_test: ssg-test.cc systemd-sonic-generator.o - $(CPP) $(CPPFLAGS) -o $@ $^ $(LFLAGS) + $(CXX) $(CXXFLAGS) -ggdb -o $@ $^ $(LDFLAGS) -systemd-sonic-generator.o: systemd-sonic-generator.c - $(CC) $(CFLAGS) -D_SSG_UNITTEST -o $@ -c $^ +systemd-sonic-generator.o: systemd-sonic-generator.cpp + $(CXX) $(CXXFLAGS) -ggdb -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 1d0f33d54..567da24ec 100644 --- a/src/systemd-sonic-generator/ssg-test.cc +++ b/src/systemd-sonic-generator/ssg-test.cc @@ -181,8 +181,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; @@ -214,7 +213,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; } @@ -460,9 +459,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); } } @@ -513,7 +511,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 69% rename from src/systemd-sonic-generator/systemd-sonic-generator.c rename to src/systemd-sonic-generator/systemd-sonic-generator.cpp index ff748ca97..d0359a4a1 100644 --- a/src/systemd-sonic-generator/systemd-sonic-generator.c +++ b/src/systemd-sonic-generator/systemd-sonic-generator.cpp @@ -1,13 +1,15 @@ -#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 @@ -48,13 +50,25 @@ 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(); + } +} + -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 @@ -101,26 +115,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; } } @@ -128,59 +148,49 @@ static bool is_multi_instance_service(char *service_name){ } -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; - 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; @@ -254,14 +264,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; @@ -269,25 +279,20 @@ int get_install_targets(char* unit_file, char* targets[]) { char* token; char* line = NULL; bool first; - char* target_suffix; - 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))) { - 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; } @@ -340,7 +345,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) { @@ -352,8 +357,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++; } @@ -374,100 +379,80 @@ int get_unit_files(char* unit_files[]) { } -char* insert_instance_number(char* unit_file, int instance) { +std::string insert_instance_number(const std::string& unit_file, int instance) { /*** 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; + 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 ""; } - /*** - suffix is "@.service", set suffix=".service" - prefix_len is length of "example@" - ***/ - prefix_len = ++suffix - unit_file; - ret = asprintf(&instance_name, "%.*s%d%s", prefix_len, unit_file, instance, suffix); - if (ret == -1) { - fprintf(stderr, "Error creating instance %d of %s\n", instance, unit_file); - return NULL; - } - - return instance_name; + return unit_file.substr(0, at_pos + 1) + std::to_string(instance) + unit_file.substr(at_pos + 1); } -static int create_symlink(char* unit, char* target, char* install_dir, int instance) { +static int create_symlink(const std::string& unit, const std::string& target, const std::string& install_dir, int instance) { 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); } - 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); - - free(unit_instance); + final_install_dir = install_dir + std::string(target); + dest_path = final_install_dir + "/" + 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; } } - 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; } @@ -476,7 +461,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 @@ -487,38 +472,34 @@ 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 { 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; @@ -531,12 +512,10 @@ int get_num_of_asic() { ***/ FILE *fp; char *line = NULL; - char* token; - char* platform; + char* platform = NULL; char* saveptr; size_t len = 0; ssize_t nread; - bool ans; char asic_file[512]; char* str_num_asic; int num_asic = 1; @@ -552,7 +531,7 @@ int get_num_of_asic() { 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); platform = strtok_r(NULL, "=", &saveptr); strip_trailing_newline(platform); break; @@ -565,7 +544,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){ @@ -584,15 +563,13 @@ int get_num_of_asic() { 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; if (argc <= 1) { fputs("Installation directory required as argument\n", stderr); @@ -600,40 +577,33 @@ int ssg_main(int argc, char **argv) { } num_asics = get_num_of_asic(); - strcpy(install_dir, argv[1]); - strcat(install_dir, "/"); + install_dir = std::string(argv[1]) + "/"; num_unit_files = get_unit_files(unit_files); // 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]); - if ((num_asics == 1) && 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 = unit_files[i]; + if ((num_asics == 1 && 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 25c179caa..febbda092 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; @@ -25,11 +27,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); +extern std::string insert_instance_number(const std::string& unit_file, int instance); 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