From 71a724189d0ca42032778815a007f71628db4a65 Mon Sep 17 00:00:00 2001 From: Thomas Rory Gummerson Date: Fri, 19 Sep 2025 13:46:21 +0200 Subject: [PATCH] Reimplement and bugfix environment_variable_path_create, portability_path_get_name, portability_path_get_name_canonical - Get rid of some buffer overruns - make more functions return more sensible things on NULL arguments - properly DOCUMENT each function - Add const type qualifiers where applicable - Replace some string functions and loops with memcpy --- .../environment/environment_variable_path.h | 26 ++- .../source/environment_variable_path.c | 69 +++----- .../include/portability/portability_path.h | 50 +++++- source/portability/source/portability_path.c | 156 ++++++++---------- .../source/portability_path_test.cpp | 55 ++++++ 5 files changed, 213 insertions(+), 143 deletions(-) diff --git a/source/environment/include/environment/environment_variable_path.h b/source/environment/include/environment/environment_variable_path.h index d5492d8f6..58b8c2047 100644 --- a/source/environment/include/environment/environment_variable_path.h +++ b/source/environment/include/environment/environment_variable_path.h @@ -52,7 +52,31 @@ extern "C" { /* -- Methods -- */ -ENVIRONMENT_API char *environment_variable_path_create(const char *name, const char *default_path, size_t default_path_size, size_t *env_size); +/** + * @brief + * If the value of `name` exists as an environment variable, return a live string of its value, otherwise return a live value of `default_path` or "/". + * + * `name` should not be `NULL`. + * + * If `default_path` is not `NULL`, `default_path_size` must be set to <= the length (including 0-terminator) of the `default_path` string. + * + * If `env_size` is not `NULL`, the length (including 0-terminator) of the returned string will be set to it + * @param[in] name + * The environment variable name to look up. + * + * @param[in] default_path + * If the environment variable value is not found, the value to return instead. + * + * @param[in] default_path_size + * The length (including 0-terminator) of `default_path` in chars. + * + * @param[out] env_size + * Pointer to a size_t to write the length of the returned string to (optional). + * + * @return + * The allocated string containing the environment variable value or the default or "/". + */ +ENVIRONMENT_API char *environment_variable_path_create(const char *const name, const char *const default_path, const size_t default_path_size, size_t *const env_size); ENVIRONMENT_API void environment_variable_path_destroy(char *variable_path); diff --git a/source/environment/source/environment_variable_path.c b/source/environment/source/environment_variable_path.c index 53f492dc3..f36439a72 100644 --- a/source/environment/source/environment_variable_path.c +++ b/source/environment/source/environment_variable_path.c @@ -43,61 +43,32 @@ /* -- Methods -- */ -char *environment_variable_path_create(const char *name, const char *default_path, size_t default_path_size, size_t *env_size) +char *environment_variable_path_create(const char *const name, const char *const default_path, const size_t default_path_size, size_t *const env_size) { - const char *path_ptr = getenv(name); - char *path; - size_t length, size, last, end; - - if (path_ptr == NULL) - { - if (default_path == NULL) - { - static const char empty_path[] = ""; - - default_path = empty_path; - default_path_size = sizeof(empty_path); - } - - path_ptr = default_path; - length = default_path_size - 1; - } - else - { - length = strlen(path_ptr); - } - - last = length - 1; - - if (ENVIRONMENT_VARIABLE_PATH_SEPARATOR(path_ptr[last])) + const char *value = getenv(name); + size_t size; + if (value) + size = strlen(value) + 1; + else if (default_path) { - end = length; - size = length + 1; + value = default_path; + size = default_path_size; } else { - last = length; - end = length + 1; - size = length + 2; + value = ENVIRONMENT_VARIABLE_PATH_SEPARATOR_STR; + size = sizeof(ENVIRONMENT_VARIABLE_PATH_SEPARATOR_STR); } - - path = malloc(sizeof(char) * size); - - if (path == NULL) - { - return NULL; - } - - strncpy(path, path_ptr, length); - - path[last] = ENVIRONMENT_VARIABLE_PATH_SEPARATOR_C; - path[end] = '\0'; - - if (env_size != NULL) - { - *env_size = size; - } - + size_t alloc_size = size; + if (size > 1) + alloc_size += !ENVIRONMENT_VARIABLE_PATH_SEPARATOR(value[size - 2]); + char *const path = malloc(sizeof(char) * alloc_size); + memcpy(path, value, sizeof(char) * size); + if (size > 1) + path[alloc_size - 2] = ENVIRONMENT_VARIABLE_PATH_SEPARATOR_C; + path[alloc_size - 1] = '\0'; + if (env_size) + *env_size = alloc_size; return path; } diff --git a/source/portability/include/portability/portability_path.h b/source/portability/include/portability/portability_path.h index 79ff0761b..100b129f1 100644 --- a/source/portability/include/portability/portability_path.h +++ b/source/portability/include/portability/portability_path.h @@ -109,9 +109,55 @@ extern "C" { /* -- Methods -- */ -PORTABILITY_API size_t portability_path_get_name(const char *path, size_t path_size, char *name, size_t name_size); +/** + * @brief + * Get the file name portion out of a path and strip away the file extension if any. + * + * If `path` is NULL this will return an empty string. + * + * If `name` is NULL or `name_size is 0 this will return the size it requires in order to write `name`. + * + * If `path` or `name` are not NULL, then `path_size` or `name_size`, respectively, must be set to <= the length (including 0-terminator) of the memory regions pointed to by `path` and `name`. + * @param[in] path + * The full path to extract the name from. + * @param[in] path_size + * The length (including 0-terminator) of `path` in chars. + * @param[out] name + * The memory location to write the extracted name to. If `NULL` the size required will be returned instead of the size written. + * @param[in] name_size + * The size of the memory location pointed to by `name`. + * @return + * The size of the name. + */ +PORTABILITY_API size_t portability_path_get_name(const char *const path, const size_t path_size, char *const name, const size_t name_size); -PORTABILITY_API size_t portability_path_get_name_canonical(const char *path, size_t path_size, char *name, size_t name_size); +/** + * @brief + * Get the file name portion out of a path and strip away any amount of file extensions. + * + * When called with `"/foo/bar.baz.qux"`: + * + * - `portability_path_get_name` will produce the string `"bar.baz"` + * + * - `portability_path_get_name_canonical` will produce the string `"bar"` + * + * If `path` is NULL this will return an empty string. + * + * If `name` is NULL or `name_size is 0 this will return the size it requires in order to write `name`. + * + * If `path` or `name` are not NULL, then `path_size` or `name_size`, respectively, must be set to <= the length (including 0-terminator) of the memory regions pointed to by `path` and `name`. + * @param[in] path + * The full path to extract the name from. + * @param[in] path_size + * The length (including 0-terminator) of `path` in chars. + * @param[out] name + * The memory location to write the extracted name to. If `NULL` the size required will be returned instead of the size written. + * @param[in] name_size + * The size of the memory location pointed to by `name`. + * @return + * The size of the name. + */ +PORTABILITY_API size_t portability_path_get_name_canonical(const char *const path, const size_t path_size, char *const name, const size_t name_size); PORTABILITY_API size_t portability_path_get_fullname(const char *path, size_t path_size, char *name, size_t name_size); diff --git a/source/portability/source/portability_path.c b/source/portability/source/portability_path.c index 9606a7d69..096df995e 100644 --- a/source/portability/source/portability_path.c +++ b/source/portability/source/portability_path.c @@ -25,107 +25,81 @@ /* Define separator checking for any platform */ #define PORTABILITY_PATH_SEPARATOR_ALL(chr) (chr == '\\' || chr == '/') -size_t portability_path_get_name(const char *path, size_t path_size, char *name, size_t name_size) +static size_t basename_offset(const char *const path, const size_t path_size) { - size_t i, count, last; - - if (path == NULL || name == NULL) - { - return 0; - } - - for (i = 0, count = 0, last = 0; path[i] != '\0' && i < path_size && count < name_size; ++i) - { - name[count++] = path[i]; - - if (PORTABILITY_PATH_SEPARATOR(path[i])) - { - count = 0; - } - else if (path[i] == '.') - { - if (i > 0 && path[i - 1] == '.') - { - last = 0; - count = 0; - } - else - { - if (count > 0) - { - last = count - 1; - } - else - { - last = 0; - } - } - } - } + size_t offset = path_size; + for (; offset != 0; offset--) + if (PORTABILITY_PATH_SEPARATOR(path[offset - 1])) + break; + return offset; +} - if ((last == 0 && count > 1) || last > count) +size_t portability_path_get_name(const char *const path, const size_t path_size, char *const name, const size_t name_size) +{ + if (path == NULL) { - last = count; + if (name == NULL || name_size == 0) + return 0; + name[0] = '\0'; + return 1; } - - name[last] = '\0'; - - return last + 1; + // find rightmost path separator + const size_t name_start = basename_offset(path, path_size); + // Find rightmost dot + size_t rightmost_dot = path_size; + for (; rightmost_dot != name_start; rightmost_dot--) + if (path[rightmost_dot - 1] == '.') + break; + // No dots found, or name starts with dot and is non-empty, use whole name + if (rightmost_dot == name_start || (rightmost_dot == name_start + 1 && rightmost_dot != path_size - 1)) + rightmost_dot = path_size - 1; + // remove all consecutive dots at the end + while (rightmost_dot != name_start && path[rightmost_dot - 1] == '.') + rightmost_dot--; + const size_t length = rightmost_dot - name_start; + const size_t size = length + 1; + // Return required size + if (name == NULL || size > name_size) + return size; + if (length) + memcpy(name, path + name_start, length); + name[length] = '\0'; + return size; } -size_t portability_path_get_name_canonical(const char *path, size_t path_size, char *name, size_t name_size) +size_t portability_path_get_name_canonical(const char *const path, const size_t path_size, char *const name, const size_t name_size) { - if (path == NULL || name == NULL) + if (path == NULL) { - return 0; + if (name == NULL || name_size == 0) + return 0; + name[0] = '\0'; + return 1; } - - size_t i, count, last; - - for (i = 0, count = 0, last = 0; path[i] != '\0' && i < path_size && count < name_size; ++i) - { - name[count++] = path[i]; - - if (PORTABILITY_PATH_SEPARATOR(path[i])) - { - count = 0; - } - else if (path[i] == '.') - { - if (i > 0 && path[i - 1] == '.') - { - last = 0; - count = 0; - } - else - { - if (count > 0) - { - last = count - 1; - } - else - { - last = 0; - } - - /* This function is the same as portability_path_get_name but - returns the name of the file without any extension, for example: - - portability_path_get_name of libnode.so.72 is libnode.so - - portability_path_get_name_canonical of libnode.so.72 is libnode - */ + // find rightmost path separator + const size_t name_start = basename_offset(path, path_size); + // find leftmost dot + size_t leftmost_dot = name_start; + for (; leftmost_dot < path_size; leftmost_dot++) + if (path[leftmost_dot] == '.') + break; + // No dots found, use whole name + if (leftmost_dot == path_size) + leftmost_dot--; + // name starts with dot, use the following dot instead + if (leftmost_dot == name_start) + for (leftmost_dot = name_start + 1; leftmost_dot < path_size; leftmost_dot++) + if (path[leftmost_dot] == '.') break; - } - } - } - - if (last == 0 && count > 1) - { - last = count; - } - - name[last] = '\0'; - - return last + 1; + const size_t length = leftmost_dot - name_start; + const size_t size = length + 1; + // Return required size + if (name == NULL || size > name_size) + return size; + if (length) + memcpy(name, path + name_start, length); + name[length] = '\0'; + return size; } size_t portability_path_get_fullname(const char *path, size_t path_size, char *name, size_t name_size) diff --git a/source/tests/portability_path_test/source/portability_path_test.cpp b/source/tests/portability_path_test/source/portability_path_test.cpp index 15534b235..f4c638c2f 100644 --- a/source/tests/portability_path_test/source/portability_path_test.cpp +++ b/source/tests/portability_path_test/source/portability_path_test.cpp @@ -68,6 +68,33 @@ TEST_F(portability_path_test, portability_path_test_path_get_module_name_with_ra EXPECT_EQ((char)'\0', (char)result[size - 1]); } +TEST_F(portability_path_test, portability_path_test_path_get_name_null) +{ + static const char result[] = ""; + + string_name name; + + size_t size = portability_path_get_name(NULL, 0, name, NAME_SIZE); + + EXPECT_STREQ(name, result); + EXPECT_EQ((size_t)size, (size_t)sizeof(result)); + EXPECT_EQ((char)'\0', (char)result[size - 1]); +} + +TEST_F(portability_path_test, portability_path_test_path_get_name_empty) +{ + static const char base[] = ""; + static const char result[] = ""; + + string_name name; + + size_t size = portability_path_get_name(base, sizeof(base), name, NAME_SIZE); + + EXPECT_STREQ(name, result); + EXPECT_EQ((size_t)size, (size_t)sizeof(result)); + EXPECT_EQ((char)'\0', (char)result[size - 1]); +} + TEST_F(portability_path_test, portability_path_test_path_get_name) { static const char base[] = "/a/b/c/asd.txt"; @@ -110,6 +137,34 @@ TEST_F(portability_path_test, portability_path_test_path_get_name_without_dot) EXPECT_EQ((char)'\0', (char)result[size - 1]); } +TEST_F(portability_path_test, portability_path_test_path_get_name_dot_in_path) +{ + static const char base[] = "/a/b.c/d/asd"; + static const char result[] = "asd"; + + string_name name; + + size_t size = portability_path_get_name(base, sizeof(base), name, NAME_SIZE); + + EXPECT_STREQ(name, result); + EXPECT_EQ((size_t)size, (size_t)sizeof(result)); + EXPECT_EQ((char)'\0', (char)result[size - 1]); +} + +TEST_F(portability_path_test, portability_path_test_path_get_name_dot_in_path_and_name) +{ + static const char base[] = "/a/b.c/d/asd.txt"; + static const char result[] = "asd"; + + string_name name; + + size_t size = portability_path_get_name(base, sizeof(base), name, NAME_SIZE); + + EXPECT_STREQ(name, result); + EXPECT_EQ((size_t)size, (size_t)sizeof(result)); + EXPECT_EQ((char)'\0', (char)result[size - 1]); +} + TEST_F(portability_path_test, portability_path_test_path_get_name_only_separator_dot) { static const char base[] = "/.";