Review of Thomas PR.

Author: viferga

source/environment/include/environment/environment_variable_path.h

@@ -54,21 +54,19 @@ extern "C" {
 
 /**
  * @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 "/".
+ *   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 null-terminator) of the @default_path string.
+ *   If @env_size is not NULL, the length (including null-terminator) of the returned string will be set to it.
  *
- *   `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.
+ *   The length (including null-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).

source/environment/source/environment_variable_path.c

@@ -45,30 +45,53 @@
 
 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 *value = getenv(name);
-	size_t size;
-	if (value)
-		size = strlen(value) + 1;
+	const char *env_variable = getenv(name);
+	char *path;
+	size_t size, alloc_size;
+
+	if (env_variable)
+	{
+		size = strlen(env_variable) + 1;
+	}
 	else if (default_path)
 	{
-		value = default_path;
+		env_variable = default_path;
 		size = default_path_size;
 	}
 	else
 	{
-		value = ENVIRONMENT_VARIABLE_PATH_SEPARATOR_STR;
+		env_variable = ENVIRONMENT_VARIABLE_PATH_SEPARATOR_STR;
 		size = sizeof(ENVIRONMENT_VARIABLE_PATH_SEPARATOR_STR);
 	}
-	size_t alloc_size = size;
+
+	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);
+	{
+		alloc_size += !ENVIRONMENT_VARIABLE_PATH_SEPARATOR(env_variable[size - 2]);
+	}
+
+	path = malloc(sizeof(char) * alloc_size);
+
+	if (path == NULL)
+	{
+		return NULL;
+	}
+
+	memcpy(path, env_variable, 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;
 }
 

source/portability/include/portability/portability_path.h

@@ -112,20 +112,22 @@ extern "C" {
 /**
  * @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 null-terminator) of the memory regions pointed to by @path and @name.
  *
- *   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.
+ *   The length (including null-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.
+ *   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`.
+ *   The size of the memory location pointed to by @name.
+ *
  * @return
  *   The size of the name.
  */
@@ -134,26 +136,27 @@ PORTABILITY_API size_t portability_path_get_name(const char *const path, const s
 /**
  * @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 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 null-terminator) of the memory regions pointed to by @path and @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.
+ *   The length (including null-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.
+ *   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`.
+ *   The size of the memory location pointed to by @name.
+ *
  * @return
  *   The size of the name.
  */

source/portability/source/portability_path.c

@@ -25,80 +25,135 @@
 /* Define separator checking for any platform */
 #define PORTABILITY_PATH_SEPARATOR_ALL(chr) (chr == '\\' || chr == '/')
 
-static size_t basename_offset(const char *const path, const size_t path_size)
+static size_t portability_path_basename_offset(const char *const path, const size_t path_size)
 {
 	size_t offset = path_size;
-	for (; offset != 0; offset--)
-		if (PORTABILITY_PATH_SEPARATOR(path[offset - 1]))
-			break;
+
+	while (offset > 0 && !PORTABILITY_PATH_SEPARATOR(path[offset - 1]))
+	{
+		--offset;
+	}
+
 	return offset;
 }
 
 size_t portability_path_get_name(const char *const path, const size_t path_size, char *const name, const size_t name_size)
 {
+	size_t name_start, rightmost_dot, length, size;
+
 	if (path == NULL)
 	{
 		if (name == NULL || name_size == 0)
+		{
 			return 0;
+		}
+
 		name[0] = '\0';
+
 		return 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
+
+	/* Find rightmost path separator */
+	name_start = portability_path_basename_offset(path, path_size);
+
+	/* Find rightmost dot */
+	rightmost_dot = path_size;
+
+	while (rightmost_dot != name_start && path[rightmost_dot - 1] != '.')
+	{
+		--rightmost_dot;
+	}
+
+	/* 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
+	}
+
+	/* 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
+	{
+		--rightmost_dot;
+	}
+
+	length = rightmost_dot - name_start;
+	size = length + 1;
+
+	/* Return required size */
 	if (name == NULL || size > name_size)
+	{
 		return size;
-	if (length)
+	}
+
+	if (length > 0)
+	{
 		memcpy(name, path + name_start, length);
+	}
+
 	name[length] = '\0';
+
 	return 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)
 {
+	size_t name_start, leftmost_dot, length, size;
+
 	if (path == NULL)
 	{
 		if (name == NULL || name_size == 0)
+		{
 			return 0;
+		}
+
 		name[0] = '\0';
+
 		return 1;
 	}
-	// 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
+
+	/* Find rightmost path separator */
+	name_start = portability_path_basename_offset(path, path_size);
+
+	/* Find leftmost dot */
+	leftmost_dot = name_start;
+
+	while (leftmost_dot < path_size && path[leftmost_dot] != '.')
+	{
+		++leftmost_dot;
+	}
+
+	/* No dots found, use whole name */
 	if (leftmost_dot == path_size)
-		leftmost_dot--;
-	// name starts with dot, use the following dot instead
+	{
+		--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;
-	const size_t length = leftmost_dot - name_start;
-	const size_t size = length + 1;
-	// Return required size
+	{
+		do
+		{
+			++leftmost_dot;
+
+		} while (leftmost_dot < path_size && path[leftmost_dot] != '.');
+	}
+
+	length = leftmost_dot - name_start;
+	size = length + 1;
+
+	/* Return required size */
 	if (name == NULL || size > name_size)
+	{
 		return size;
-	if (length)
+	}
+
+	if (length > 0)
+	{
 		memcpy(name, path + name_start, length);
+	}
+
 	name[length] = '\0';
+
 	return size;
 }