Review comments

Author: dpw13

include/core/path_utils.h

@@ -2,7 +2,7 @@
 #define LIGHTNVR_PATH_UTILS_H
 
 #include <stddef.h>
-#include <fcntl.h>
+#include <sys/types.h>
 
 /**
  * Sanitize a stream name for use as a path or object name.
@@ -16,9 +16,10 @@ void sanitize_stream_name(const char *input, char *output, size_t output_size);
 
 /**
  * Ensure the specified directory exists, creating it if necessary. Does not recur.
+ * Sets errno on failure.
  *
  * @param path Directory path to create
- * @return 0 on success, -1 on error
+ * @return 0 on success, negative errno on error
  */
 int ensure_dir(const char *path);
 

src/core/daemon.c

@@ -198,6 +198,7 @@ static void daemon_signal_handler(int sig) {
 int write_pid_file(const char *pid_file) {
     // Make sure the directory exists
     if (ensure_path(pid_file) != 0) {
+        log_error("Failed to create PID file directory %s: %s", pid_file, strerror(errno));
         return -1;
     }
 

src/core/path_utils.c

@@ -1,14 +1,12 @@
 #include <ctype.h>
 #include <dirent.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <libgen.h>
+#include <limits.h>
 #include <stdbool.h>
 #include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <strings.h>
 #include <sys/stat.h>
-#include <sys/types.h>
 #include <unistd.h>
 
 #include "core/path_utils.h"
@@ -38,14 +36,16 @@ int ensure_dir(const char *path) {
         if (!S_ISDIR(st.st_mode)) {
             // Path exists but is not a directory
             log_error("Regular file in the way of directory at %s", path);
-            return -1;
+            // Also set errno
+            errno = ENOTDIR;
+            return -ENOTDIR;
         }
-        // If path exists and is directory, fall through to update p
+        // If path exists and is directory, we're done
     } else {
         // Create this directory level
         if (mkdir(path, 0755) != 0 && errno != EEXIST) {
             log_error("Failed to create directory %s: %s", path, strerror(errno));
-            return -1;
+            return -errno;
         }
     }
 
@@ -56,9 +56,9 @@ int ensure_dir(const char *path) {
  * Create a directory recursively (like mkdir -p), internal implementation.
  * 
  * Will modify path in-place, but restores it before returning. Does not perform
- * input validation.
+ * input validation. Returns the negative error code if one is encountered.
  */
-int _mkdir_recursive(char *path) {
+static int _mkdir_recursive(char *path) {
     // Iterate through path components and create each directory
     char *p = path;
 
@@ -126,7 +126,7 @@ int mkdir_recursive(const char *path) {
  * Set permissions on a file or directory (like chmod). Sets fd_out if the
  * path is a directory for use in recursive chmod.
  */
-int _chmod_path(const char *path, mode_t mode, int *fd_out) {
+static int _chmod_path(const char *path, mode_t mode, int *fd_out) {
     // Open as a directory first (O_NOFOLLOW prevents following symlinks, eliminating
     // the TOCTOU race between the old lstat() check and chmod() on the same path).
     int fd = open(path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
@@ -223,6 +223,13 @@ int chmod_recursive(const char *path, mode_t mode) {
     int result = 0;
 
     size_t offset = snprintf(full_path, sizeof(full_path), "%s/", path);
+    if (offset >= PATH_MAX) {
+        // If the path is already max length, we won't be able to append anything to
+        // it. In this case emit an error and return.
+        log_error("Path too long to recur for chmod");
+        close(fd);
+        return -1;
+    }
     char *path_ptr = full_path + offset;
 
     while ((entry = readdir(dir)) != NULL) {

src/video/hls_writer.c

@@ -414,7 +414,7 @@ static int ensure_output_directory(hls_writer_t *writer) {
 
     // Create directory if necessary
     if (mkdir_recursive(safe_dir_path)) {
-        log_error("Failed to create HLS output directory");
+        log_error("Failed to create HLS output directory %s: %s", safe_dir_path, strerror(errno));
         return -1;
     }
 

src/video/packet_buffer.c

@@ -265,7 +265,13 @@ packet_buffer_t* create_packet_buffer(const char *stream_name, int buffer_second
         if (config) {
             snprintf(buffer->disk_buffer_path, sizeof(buffer->disk_buffer_path),
                     "%s/.packet_buffer_%s", config->storage_path, stream_name);
-            ensure_dir(buffer->disk_buffer_path);
+            if (ensure_dir(buffer->disk_buffer_path)) {
+                log_error("Failed to create disk buffer directory %s: %s", buffer->disk_buffer_path, strerror(errno));
+                pthread_mutex_destroy(&buffer->mutex);
+                buffer->mutex_initialized = false;
+                pthread_mutex_unlock(&buffer_pool.pool_mutex);
+                return NULL;
+            }
         }
     }
 

src/video/unified_detection_thread.c

@@ -507,7 +507,12 @@ int start_unified_detection_thread(const char *stream_name, const char *model_pa
 
         snprintf(ctx->output_dir, sizeof(ctx->output_dir), "%s/%s",
                  global_cfg->storage_path, stream_path);
-        ensure_dir(ctx->output_dir);
+        if (ensure_dir(ctx->output_dir)) {
+            log_error("Failed to create output directory %s: %s", ctx->output_dir, strerror(errno));
+            free(ctx);
+            pthread_mutex_unlock(&contexts_mutex);
+            return -1;
+        }
     }
 
     // If using built-in motion detection, enable the motion stream now so that

src/web/api_handlers_system.c

@@ -1442,7 +1442,11 @@ void handle_post_system_backup(const http_request_t *req, http_response_t *res)
     snprintf(backup_path, sizeof(backup_path), "%s/backups", g_config.web_root);
 
     // Create backups directory if it doesn't exist
-    ensure_dir(backup_path);
+    if (ensure_dir(backup_path)) {
+        log_error("Failed to create backup directory %s: %s", backup_path, strerror(errno));
+        http_response_set_json_error(res, 500, "Failed to create backup directory");
+        return;
+    }
 
     // Append filename to path
     snprintf(backup_path, sizeof(backup_path), "%s/backups/%s", g_config.web_root, backup_filename);

src/web/api_handlers_timeline.c

@@ -587,7 +587,10 @@ int create_timeline_manifest(const timeline_segment_t *segments, int segment_cou
     snprintf(temp_dir, sizeof(temp_dir), "%s/timeline_manifests", g_config.storage_path);
 
     // Create directory if it doesn't exist
-    ensure_dir(temp_dir);
+    if (ensure_dir(temp_dir)) {
+        log_error("Could not create directory for timeline manifest %s: %s", temp_dir, strerror(errno));
+        return -1;
+    }
 
     // Generate a unique manifest filename
     char manifest_filename[MAX_PATH_LENGTH];