From d43f44773f1f9581b944f01c578977f4311b4f47 Mon Sep 17 00:00:00 2001 From: Dane Wagner Date: Fri, 10 Apr 2026 13:49:04 -0500 Subject: [PATCH 1/3] Standardize mkdir and chmod functionality * Move mkdir and chmod functionality from ffmpeg_utils to path_utils * Refactor mkdir_recursive and add ensure_path and ensure_dir * Refactor chmod to be a bit safer and re-use more code * Add chmod_parent --- include/core/path_utils.h | 56 +++++++++ include/video/ffmpeg_utils.h | 27 ----- src/core/path_utils.c | 228 ++++++++++++++++++++++++++++++++++- src/video/ffmpeg_utils.c | 141 ---------------------- 4 files changed, 282 insertions(+), 170 deletions(-) diff --git a/include/core/path_utils.h b/include/core/path_utils.h index 3410cddf..e7a47c96 100644 --- a/include/core/path_utils.h +++ b/include/core/path_utils.h @@ -2,6 +2,7 @@ #define LIGHTNVR_PATH_UTILS_H #include +#include /** * Sanitize a stream name for use as a path or object name. @@ -13,4 +14,59 @@ */ 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. + * + * @param path Directory path to create + * @return 0 on success, -1 on error + */ +int ensure_dir(const char *path); + +/** + * Create a directory recursively (like mkdir -p) + * This replaces system("mkdir -p ...") calls + * + * @param path Directory path to create + * @return 0 on success, -1 on error + */ +int mkdir_recursive(const char *path); + +/** + * Ensure a directory exists for a file path by recursively creating + * parent directories. + * + * @param path File path to create parent directories for + * @return 0 on success, -1 on error + */ +int ensure_path(const char *path); + +/** + * Set permissions on a file or directory (like chmod) + * + * @param path Path to set permissions on + * @param mode Permission mode (e.g., 0755) + * @return 0 on success, -1 on error + */ +int chmod_path(const char *path, mode_t mode); + +/** + * Set permissions on a parent directory + * + * @param path Path to the file inside the parent directory + * @param mode Permission mode (e.g., 0755) + * @return 0 on success, -1 on error + */ +int chmod_parent(const char *path, mode_t mode); + +/** + * Recursively set permissions on a directory and its contents (like chmod -R) + * + * @param path Directory path to chmod recursively + * @param mode Permission mode (e.g., 0755) + * @return 0 on success, -1 on error + */ +int chmod_recursive(const char *path, mode_t mode); + +// TODO: create recursive chmod for folders only and use that instead of chmod_recursive + #endif /* LIGHTNVR_PATH_UTILS_H */ \ No newline at end of file diff --git a/include/video/ffmpeg_utils.h b/include/video/ffmpeg_utils.h index 030f5da9..fd4f0a79 100644 --- a/include/video/ffmpeg_utils.h +++ b/include/video/ffmpeg_utils.h @@ -157,31 +157,4 @@ void jpeg_encoder_cleanup_all(void); int ffmpeg_concat_ts_to_mp4(const char **segment_paths, int segment_count, const char *output_path); -/** - * Create a directory recursively (like mkdir -p) - * This replaces system("mkdir -p ...") calls - * - * @param path Directory path to create - * @return 0 on success, -1 on error - */ -int mkdir_recursive(const char *path); - -/** - * Set permissions on a file or directory (like chmod) - * - * @param path Path to set permissions on - * @param mode Permission mode (e.g., 0777) - * @return 0 on success, -1 on error - */ -int chmod_path(const char *path, mode_t mode); - -/** - * Recursively set permissions on a directory and its contents (like chmod -R) - * - * @param path Directory path to chmod recursively - * @param mode Permission mode (e.g., 0777) - * @return 0 on success, -1 on error - */ -int chmod_recursive(const char *path, mode_t mode); - #endif /* FFMPEG_UTILS_H */ diff --git a/src/core/path_utils.c b/src/core/path_utils.c index 53768fae..bd9a403d 100644 --- a/src/core/path_utils.c +++ b/src/core/path_utils.c @@ -1,10 +1,19 @@ -#include "core/path_utils.h" - #include +#include +#include +#include +#include #include #include #include #include +#include +#include +#include + +#include "core/path_utils.h" +#include "core/logger.h" +#include "utils/strings.h" void sanitize_stream_name(const char *input, char *output, size_t output_size) { size_t i = 0; @@ -20,3 +29,218 @@ void sanitize_stream_name(const char *input, char *output, size_t output_size) { } output[i] = '\0'; } + +// Ensure the specified directory exists, creating it if necessary. Does not recur. +int ensure_dir(const char *path) { + struct stat st; + + if (stat(path, &st) == 0) { + 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; + } + // If path exists and is directory, fall through to update p + } 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 0; +} + +/** + * Create a directory recursively (like mkdir -p), internal implementation. + * + * Will modify path in-place, but restores it before returning. Does not perform + * input validation. + */ +int _mkdir_recursive(char *path) { + // Iterate through path components and create each directory + char *p = path; + + // Skip leading slash for absolute paths + if (*p == '/') { + p++; + } + + while (*p) { + // Find next slash + while (*p && *p != '/') { + p++; + } + + // Terminate string at directory separator + char saved = *p; + *p = '\0'; + + // Check if directory already exists + if (ensure_dir(path)) { + return -1; + } + + *p = saved; + if (*p) { + p++; + } + } + + return 0; +} + +/** + * Ensure a directory exists for a file path + */ +int ensure_path(const char *path) { + if (!path || !*path) { + return -1; + } + + // Use the full OS path length here since we'll be calling `dirname`. We + // need a writable copy of path since dirname may modify the string. + char path_buf[PATH_MAX]; + + safe_strcpy(path_buf, path, PATH_MAX, 0); + char *dir = dirname(path_buf); + return _mkdir_recursive(dir); +} + +/** + * Create a directory recursively (like mkdir -p). + */ +int mkdir_recursive(const char *path) { + if (!path || !*path) { + return -1; + } + + // Make a mutable copy of the path + char path_copy[PATH_MAX]; + safe_strcpy(path_copy, path, PATH_MAX, 0); + return _mkdir_recursive(path_copy); +} + +/** + * 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) { + // 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); + bool is_dir = (fd >= 0); + + if (!is_dir) { + if (errno == ENOTDIR) { + // Not a directory — open as a regular file, still with O_NOFOLLOW + fd = open(path, O_RDONLY | O_NOFOLLOW); + } + if (fd < 0) { + if (errno == ELOOP) { + // Path is a symlink — skip without error (don't follow) + return 0; + } + log_warn("Failed to open %s: %s", path, strerror(errno)); + return -1; + } + } + + // Apply permissions atomically through the open fd (no TOCTOU) + if (fchmod(fd, mode) != 0) { + log_warn("Failed to fchmod %s: %s", path, strerror(errno)); + close(fd); + return -1; + } + + if (!is_dir || fd_out == NULL) { + close(fd); + fd = -1; + } + + if (fd_out) { + *fd_out = fd; + } + + return 0; +} + +/** + * Set permissions on a file or directory (like chmod) + */ +int chmod_path(const char *path, mode_t mode) { + if (!path || !*path) { + return -1; + } + + return _chmod_path(path, mode, NULL); +} + +int chmod_parent(const char *path, mode_t mode) { + if (!path || !*path) { + return -1; + } + + // Use the full OS path length here since we'll be calling `dirname`. We + // need a writable copy of path since dirname may modify the string. + char path_buf[PATH_MAX]; + + safe_strcpy(path_buf, path, PATH_MAX, 0); + char *dir = dirname(path_buf); + return _chmod_path(dir, mode, NULL); +} + +/** + * Recursively set permissions on a directory and its contents (like chmod -R) + */ +int chmod_recursive(const char *path, mode_t mode) { + if (!path || !*path) { + return -1; + } + + int fd; + if (_chmod_path(path, mode, &fd)) { + return -1; + } + + if (fd < 0) { + // Path was a file, we're done. + return 0; + } + + // Directory: use fdopendir so readdir operates on the already-open fd + DIR *dir = fdopendir(fd); + if (!dir) { + log_warn("Failed to fdopendir %s: %s", path, strerror(errno)); + close(fd); + return -1; + } + // fd is now owned by dir; do not close it separately + + const struct dirent *entry; + char full_path[PATH_MAX]; + int result = 0; + + size_t offset = snprintf(full_path, sizeof(full_path), "%s/", path); + char *path_ptr = full_path + offset; + + while ((entry = readdir(dir)) != NULL) { + // Skip . and .. + if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) { + continue; + } + + // Overwrite just the file portion of the path + safe_strcpy(path_ptr, entry->d_name, sizeof(full_path) - offset, 0); + + // Recursively chmod (opens each entry with O_NOFOLLOW) + if (chmod_recursive(full_path, mode) != 0) { + result = -1; + // Continue processing other entries + } + } + + closedir(dir); // also closes fd + return result; +} diff --git a/src/video/ffmpeg_utils.c b/src/video/ffmpeg_utils.c index c97515a1..f2f2293b 100644 --- a/src/video/ffmpeg_utils.c +++ b/src/video/ffmpeg_utils.c @@ -4,8 +4,6 @@ #include "video/ffmpeg_leak_detector.h" #include "video/stream_protocol.h" -#include -#include #include #include #include @@ -336,145 +334,6 @@ int periodic_ffmpeg_reset(AVFormatContext **input_ctx_ptr, const char *url, int return 0; } -/** - * Create a directory recursively (like mkdir -p) - */ -int mkdir_recursive(const char *path) { - if (!path || !*path) { - return -1; - } - - // Make a mutable copy of the path - char *path_copy = strdup(path); - if (!path_copy) { - log_error("Failed to allocate memory for path copy"); - return -1; - } - - // Iterate through path components and create each directory - char *p = path_copy; - - // Skip leading slash for absolute paths - if (*p == '/') { - p++; - } - - while (*p) { - // Find next slash - while (*p && *p != '/') { - p++; - } - - char saved = *p; - *p = '\0'; - - // Create this directory level - if (mkdir(path_copy, 0755) != 0 && errno != EEXIST) { - log_error("Failed to create directory %s: %s", path_copy, strerror(errno)); - free(path_copy); - return -1; - } - - *p = saved; - if (*p) { - p++; - } - } - - free(path_copy); - return 0; -} - -/** - * Set permissions on a file or directory (like chmod) - */ -int chmod_path(const char *path, mode_t mode) { - if (!path || !*path) { - return -1; - } - - if (chmod(path, mode) != 0) { - log_warn("Failed to chmod %s: %s", path, strerror(errno)); - return -1; - } - - return 0; -} - -/** - * Recursively set permissions on a directory and its contents (like chmod -R) - * Note: This is a simplified version that only sets permissions on the directory itself - * For full recursive chmod, we would need to walk the directory tree - */ -int chmod_recursive(const char *path, mode_t mode) { - if (!path || !*path) { - return -1; - } - - // 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); - bool is_dir = (fd >= 0); - - if (!is_dir) { - if (errno == ENOTDIR) { - // Not a directory — open as a regular file, still with O_NOFOLLOW - fd = open(path, O_RDONLY | O_NOFOLLOW); - } - if (fd < 0) { - if (errno == ELOOP) { - // Path is a symlink — skip without error (don't follow) - return 0; - } - log_warn("Failed to open %s: %s", path, strerror(errno)); - return -1; - } - } - - // Apply permissions atomically through the open fd (no TOCTOU) - if (fchmod(fd, mode) != 0) { - log_warn("Failed to fchmod %s: %s", path, strerror(errno)); - close(fd); - return -1; - } - - if (!is_dir) { - close(fd); - return 0; - } - - // Directory: use fdopendir so readdir operates on the already-open fd - DIR *dir = fdopendir(fd); - if (!dir) { - log_warn("Failed to fdopendir %s: %s", path, strerror(errno)); - close(fd); - return -1; - } - // fd is now owned by dir; do not close it separately - - const struct dirent *entry; - char full_path[PATH_MAX]; - int result = 0; - - while ((entry = readdir(dir)) != NULL) { - // Skip . and .. - if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) { - continue; - } - - snprintf(full_path, sizeof(full_path), "%s/%s", path, entry->d_name); - - // Recursively chmod (opens each entry with O_NOFOLLOW) - if (chmod_recursive(full_path, mode) != 0) { - result = -1; - // Continue processing other entries - } - } - - closedir(dir); // also closes fd - return result; -} - /** * Encode raw image data to JPEG using FFmpeg libraries */ From 913e7a8cfe25790d61c159f572076b7bda8290fd Mon Sep 17 00:00:00 2001 From: Dane Wagner Date: Sat, 11 Apr 2026 17:05:32 -0500 Subject: [PATCH 2/3] Use standardized mkdir and chmod functions * go2rtc_process: mkdir -> mkdir_recursive * daemon.c: code -> ensure_path * config, loggers, db_core: duplicate create_directory -> mkdir_recursive * main: mkdir_recursive, ensure_path * buffer_strategy_mmap: ensure_dir * storage manager: mkdir_recursive, ensure_dir * main.c path_utils.h header fixup * use chmod_path * onvif_motion_recording: mkdir->mkdir_recursive * hls_writer: code -> mkdir_recursive * Use mkdir_recursive or ensure_dir instead of direct mkdir --- src/core/config.c | 68 +++--------------- src/core/daemon.c | 20 ++---- src/core/logger.c | 45 +----------- src/core/logger_json.c | 45 +----------- src/core/main.c | 19 ++--- src/database/db_core.c | 49 +------------ src/storage/storage_manager.c | 20 ++---- src/video/buffer_strategy_mmap.c | 14 ++-- src/video/go2rtc/go2rtc_process.c | 10 ++- src/video/hls/hls_directory.c | 12 ++-- src/video/hls/hls_unified_thread.c | 6 +- src/video/hls_writer.c | 77 +++------------------ src/video/mp4_writer_utils.c | 3 +- src/video/onvif_motion_recording.c | 15 ++-- src/video/packet_buffer.c | 3 +- src/video/unified_detection_thread.c | 4 +- src/web/api_handlers_recordings_thumbnail.c | 3 +- src/web/api_handlers_system.c | 3 +- src/web/api_handlers_timeline.c | 6 +- tests/unit/test_api_handlers_system.c | 4 +- tests/unit/test_config.c | 3 +- tests/unit/test_storage_manager_retention.c | 5 +- 22 files changed, 85 insertions(+), 349 deletions(-) diff --git a/src/core/config.c b/src/core/config.c index 6b53cce5..998081da 100644 --- a/src/core/config.c +++ b/src/core/config.c @@ -16,6 +16,7 @@ #include "ini.h" #include "core/config.h" #include "core/logger.h" +#include "core/path_utils.h" #include "database/database_manager.h" #include "utils/strings.h" @@ -471,59 +472,17 @@ void load_default_config(config_t *config) { config->mqtt_ha_snapshot_interval = 30; // 30 seconds default } -// Create directory if it doesn't exist -static int create_directory(const char *path) { - struct stat st; - - // Check if directory already exists - if (stat(path, &st) == 0) { - if (S_ISDIR(st.st_mode)) { - return 0; // Directory exists - } else { - return -1; // Path exists but is not a directory - } - } - - // Create directory with permissions 0755 - if (mkdir(path, 0755) != 0) { - if (errno == ENOENT) { - // Parent directory doesn't exist, try to create it recursively - char *parent_path = strdup(path); - if (!parent_path) { - return -1; - } - - const char *parent_dir = dirname(parent_path); - int ret = create_directory(parent_dir); - free(parent_path); - - if (ret != 0) { - return -1; - } - - // Try again to create the directory - if (mkdir(path, 0755) != 0) { - return -1; - } - } else { - return -1; - } - } - - return 0; -} - // Ensure all required directories exist static int ensure_directories(const config_t *config) { // Storage directory - if (create_directory(config->storage_path) != 0) { + if (mkdir_recursive(config->storage_path) != 0) { log_error("Failed to create storage directory: %s", config->storage_path); return -1; } // HLS storage directory if specified if (config->storage_path_hls[0] != '\0') { - if (create_directory(config->storage_path_hls) != 0) { + if (mkdir_recursive(config->storage_path_hls) != 0) { log_error("Failed to create HLS storage directory: %s", config->storage_path_hls); return -1; } @@ -531,7 +490,7 @@ static int ensure_directories(const config_t *config) { } // Models directory - if (create_directory(config->models_path) != 0) { + if (mkdir_recursive(config->models_path) != 0) { log_error("Failed to create models directory: %s", config->models_path); return -1; } @@ -539,25 +498,20 @@ static int ensure_directories(const config_t *config) { // Database directory // Some dirname implementations actually modify the path argument and // will segfault when passed a read-only const string. - char tmp_path[MAX_PATH_LENGTH]; - safe_strcpy(tmp_path, config->db_path, MAX_PATH_LENGTH, 0); - char *dir = dirname(tmp_path); - if (create_directory(dir) != 0) { - log_error("Failed to create database directory: %s", dir); + if (ensure_path(config->db_path)) { + log_error("Failed to create database directory: %s", config->db_path); return -1; } // Web root directory - if (create_directory(config->web_root) != 0) { + if (mkdir_recursive(config->web_root) != 0) { log_error("Failed to create web root directory: %s", config->web_root); return -1; } // Log directory - safe_strcpy(tmp_path, config->log_file, MAX_PATH_LENGTH, 0); - dir = dirname(tmp_path); - if (create_directory(dir) != 0) { - log_error("Failed to create log directory: %s", dir); + if (ensure_path(config->log_file)) { + log_error("Failed to create log directory: %s", config->log_file); return -1; } @@ -566,7 +520,7 @@ static int ensure_directories(const config_t *config) { if (cfg_log_fd < 0) { log_warn("Log file %s is not writable: %s", config->log_file, strerror(errno)); // Try to fix log directory permissions - if (chmod(dir, 0755) != 0) { + if (chmod_parent(config->log_file, 0755)) { log_warn("Failed to change log directory permissions: %s", strerror(errno)); } } else { @@ -1452,7 +1406,7 @@ int save_config(const config_t *config, const char *path) { struct stat st; if (stat(dir_path, &st) != 0) { log_warn("Directory %s does not exist, attempting to create it", dir_path); - if (create_directory(dir_path) != 0) { + if (mkdir_recursive(dir_path) != 0) { log_error("Failed to create directory %s: %s", dir_path, strerror(errno)); return -1; } diff --git a/src/core/daemon.c b/src/core/daemon.c index ffb3221e..6ac6004c 100644 --- a/src/core/daemon.c +++ b/src/core/daemon.c @@ -17,6 +17,7 @@ #include "web/web_server.h" #include "core/logger.h" #include "core/daemon.h" +#include "core/path_utils.h" #include "utils/strings.h" extern volatile bool running; // Reference to the global variable defined in main.c @@ -196,24 +197,11 @@ static void daemon_signal_handler(int sig) { // Write PID file int write_pid_file(const char *pid_file) { // Make sure the directory exists - const char *last_slash = strrchr(pid_file, '/'); - if (last_slash) { - char dir_path[MAX_PATH_LENGTH] = {0}; - size_t dir_len = (size_t)(last_slash - pid_file); - memcpy(dir_path, pid_file, dir_len); - dir_path[dir_len] = '\0'; - - // Create directory if it doesn't exist - struct stat st; - if (stat(dir_path, &st) != 0) { - if (mkdir(dir_path, 0755) != 0 && errno != EEXIST) { - log_error("Could not create directory for PID file: %s", strerror(errno)); - return -1; - } - } + if (ensure_path(pid_file) != 0) { + return -1; } - // Open the file with exclusive locking + // Open the file initially without exclusive locking int fd = open(pid_file, O_RDWR | O_CREAT, 0644); if (fd < 0) { log_error("Failed to open PID file %s: %s", pid_file, strerror(errno)); diff --git a/src/core/logger.c b/src/core/logger.c index b1c4b139..ecfd0cad 100644 --- a/src/core/logger.c +++ b/src/core/logger.c @@ -18,6 +18,7 @@ #include "core/config.h" #include "core/logger.h" #include "core/logger_json.h" +#include "core/path_utils.h" #include "utils/strings.h" // Logger state @@ -183,48 +184,6 @@ void set_log_level(log_level_t level) { } } -// Create directory if it doesn't exist -static int create_directory(const char *path) { - struct stat st; - - // Check if directory already exists - if (stat(path, &st) == 0) { - if (S_ISDIR(st.st_mode)) { - return 0; // Directory exists - } else { - return -1; // Path exists but is not a directory - } - } - - // Create directory with permissions 0755 - if (mkdir(path, 0755) != 0) { - if (errno == ENOENT) { - // Parent directory doesn't exist, try to create it recursively - char *parent_path = strdup(path); - if (!parent_path) { - return -1; - } - - const char *parent_dir = dirname(parent_path); - int ret = create_directory(parent_dir); - free(parent_path); - - if (ret != 0) { - return -1; - } - - // Try again to create the directory - if (mkdir(path, 0755) != 0) { - return -1; - } - } else { - return -1; - } - } - - return 0; -} - // Set the log file int set_log_file(const char *filename) { if (!filename) return -1; @@ -245,7 +204,7 @@ int set_log_file(const char *filename) { } const char *dir = dirname(dir_path); - if (create_directory(dir) != 0) { + if (mkdir_recursive(dir) != 0) { free(dir_path); pthread_mutex_unlock(&logger.mutex); return -1; diff --git a/src/core/logger_json.c b/src/core/logger_json.c index fc46bede..04ffbe65 100644 --- a/src/core/logger_json.c +++ b/src/core/logger_json.c @@ -13,6 +13,7 @@ #include "core/config.h" #include "core/logger.h" #include "core/logger_json.h" +#include "core/path_utils.h" #include "utils/strings.h" #include @@ -37,48 +38,6 @@ static const char *json_log_level_strings[] = { "debug" }; -// Create directory if it doesn't exist (copied from logger.c) -static int create_directory(const char *path) { - struct stat st; - - // Check if directory already exists - if (stat(path, &st) == 0) { - if (S_ISDIR(st.st_mode)) { - return 0; // Directory exists - } else { - return -1; // Path exists but is not a directory - } - } - - // Create directory with permissions 0755 - if (mkdir(path, 0755) != 0) { - if (errno == ENOENT) { - // Parent directory doesn't exist, try to create it recursively - char *parent_path = strdup(path); - if (!parent_path) { - return -1; - } - - const char *parent_dir = dirname(parent_path); - int ret = create_directory(parent_dir); - free(parent_path); - - if (ret != 0) { - return -1; - } - - // Try again to create the directory - if (mkdir(path, 0755) != 0) { - return -1; - } - } else { - return -1; - } - } - - return 0; -} - /** * @brief Initialize the JSON logger * @@ -104,7 +63,7 @@ int init_json_logger(const char *filename) { } const char *dir = dirname(dir_path); - if (create_directory(dir) != 0) { + if (mkdir_recursive(dir) != 0) { free(dir_path); pthread_mutex_unlock(&json_logger.mutex); return -1; diff --git a/src/core/main.c b/src/core/main.c index 874f6328..55e30d37 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -25,6 +25,7 @@ #include "core/shutdown_coordinator.h" #include "core/curl_init.h" #include "core/mqtt_client.h" +#include "core/path_utils.h" #include "utils/strings.h" #include "video/stream_manager.h" #include "video/stream_state.h" @@ -671,20 +672,14 @@ int main(int argc, char *argv[]) { config.web_root, storage_web_path); // Create the directory in our storage path - if (mkdir(storage_web_path, 0755) != 0 && errno != EEXIST) { + if (mkdir_recursive(storage_web_path)) { log_error("Failed to create web root in storage path: %s", strerror(errno)); return EXIT_FAILURE; } // Create parent directory for symlink if needed - char parent_dir[MAX_PATH_LENGTH]; - safe_strcpy(parent_dir, config.web_root, sizeof(parent_dir), 0); - char *last_slash = strrchr(parent_dir, '/'); - if (last_slash) { - *last_slash = '\0'; - if (mkdir(parent_dir, 0755) != 0 && errno != EEXIST) { - log_warn("Failed to create parent directory for web root symlink: %s", strerror(errno)); - } + if (ensure_path(config.web_root)) { + log_warn("Failed to create parent directory for web root symlink"); } // Create the symlink @@ -700,8 +695,8 @@ int main(int argc, char *argv[]) { } } else { // Try to create it directly - if (mkdir(config.web_root, 0755) != 0) { - log_error("Failed to create web root directory: %s", strerror(errno)); + if (mkdir_recursive(config.web_root)) { + log_error("Failed to create web root directory"); return EXIT_FAILURE; } @@ -960,7 +955,7 @@ int main(int argc, char *argv[]) { log_error("Detection will not work properly!"); // Create the models directory if it doesn't exist - if (mkdir(config.models_path, 0755) != 0 && errno != EEXIST) { + if (mkdir_recursive(config.models_path)) { log_error("Failed to create models directory: %s", strerror(errno)); } else { log_info("Created models directory: %s", config.models_path); diff --git a/src/database/db_core.c b/src/database/db_core.c index 6545fd3f..0296db79 100644 --- a/src/database/db_core.c +++ b/src/database/db_core.c @@ -26,6 +26,7 @@ #include "database/db_backup.h" #include "core/config.h" #include "core/logger.h" +#include "core/path_utils.h" #include "utils/strings.h" // Database handle @@ -48,8 +49,6 @@ static bool wal_mode_enabled = false; // No longer tracking prepared statements - each function is responsible for finalizing its own statements -static int create_directory(const char *path); - static int sync_path_if_exists(const char *path) { int fd = open(path, O_RDONLY); if (fd < 0) { @@ -145,7 +144,7 @@ static int get_backup_directory(char *backup_dir, size_t backup_dir_size) { return -1; } - if (create_directory(backup_dir) != 0) { + if (mkdir_recursive(backup_dir) != 0) { log_error("Failed to create backup directory: %s", backup_dir); return -1; } @@ -442,48 +441,6 @@ static int perform_database_backup_cycle(const char *reason, bool run_post_backu return 0; } -// Create directory if it doesn't exist -static int create_directory(const char *path) { - struct stat st; - - // Check if directory already exists - if (stat(path, &st) == 0) { - if (S_ISDIR(st.st_mode)) { - return 0; // Directory exists - } else { - return -1; // Path exists but is not a directory - } - } - - // Create directory with permissions 0755 - if (mkdir(path, 0755) != 0) { - if (errno == ENOENT) { - // Parent directory doesn't exist, try to create it recursively - char *parent_path = strdup(path); - if (!parent_path) { - return -1; - } - - const char *parent_dir = dirname(parent_path); - int ret = create_directory(parent_dir); - free(parent_path); - - if (ret != 0) { - return -1; - } - - // Try again to create the directory - if (mkdir(path, 0755) != 0) { - return -1; - } - } else { - return -1; - } - } - - return 0; -} - // Function to checkpoint the database WAL file int checkpoint_database(void) { int rc = SQLITE_OK; @@ -690,7 +647,7 @@ int init_database(const char *db_path) { char *dir = dirname(dir_path); log_info("Creating database directory if needed: %s", dir); - if (create_directory(dir) != 0) { + if (mkdir_recursive(dir) != 0) { log_error("Failed to create database directory: %s", dir); free(dir_path); return -1; diff --git a/src/storage/storage_manager.c b/src/storage/storage_manager.c index 20eca4fb..46fc0b2c 100644 --- a/src/storage/storage_manager.c +++ b/src/storage/storage_manager.c @@ -124,14 +124,8 @@ int init_storage_manager(const char *storage_path, uint64_t max_size) { storage_manager.max_size = max_size; // Create storage directory if it doesn't exist - struct stat st; - if (stat(storage_path, &st) != 0) { - if (mkdir(storage_path, 0755) != 0) { - log_error("Failed to create storage directory: %s", strerror(errno)); - return -1; - } - } else if (!S_ISDIR(st.st_mode)) { - log_error("Storage path is not a directory: %s", storage_path); + if (mkdir_recursive(storage_path)) { + log_error("Failed to create storage directory: %s", strerror(errno)); return -1; } @@ -593,14 +587,8 @@ int create_stream_directory(const char *stream_name) { char dir_path[MAX_PATH_LENGTH]; snprintf(dir_path, sizeof(dir_path), "%s/%s", storage_manager.storage_path, encoded_name); - struct stat st; - if (stat(dir_path, &st) != 0) { - if (mkdir(dir_path, 0755) != 0) { - log_error("Failed to create stream directory: %s", strerror(errno)); - return -1; - } - } else if (!S_ISDIR(st.st_mode)) { - log_error("Stream path is not a directory: %s", dir_path); + if (ensure_dir(dir_path)) { + log_error("Failed to create stream directory"); return -1; } diff --git a/src/video/buffer_strategy_mmap.c b/src/video/buffer_strategy_mmap.c index 3664a069..5935fc14 100644 --- a/src/video/buffer_strategy_mmap.c +++ b/src/video/buffer_strategy_mmap.c @@ -166,17 +166,17 @@ static int mmap_strategy_init(pre_buffer_strategy_t *self, const buffer_config_t char safe_name[MAX_STREAM_NAME]; sanitize_stream_name(data->stream_name, safe_name, sizeof(safe_name)); - // Set up file path - snprintf(data->file_path, sizeof(data->file_path), - "%s/buffer/%s_prebuffer.mmap", - config->storage_path ? config->storage_path : g_config.storage_path, - safe_name); - // Ensure directory exists char dir_path[MAX_PATH_LENGTH]; snprintf(dir_path, sizeof(dir_path), "%s/buffer", config->storage_path ? config->storage_path : g_config.storage_path); - mkdir(dir_path, 0755); + if (ensure_dir(dir_path)) { + log_error("Failed to create directory for mmaps"); + return -1; + } + + // Set up file path + snprintf(data->file_path, sizeof(data->file_path), "%s/%s_prebuffer.mmap", dir_path, safe_name); pthread_mutex_init(&data->lock, NULL); diff --git a/src/video/go2rtc/go2rtc_process.c b/src/video/go2rtc/go2rtc_process.c index 07cb9b4f..67704108 100644 --- a/src/video/go2rtc/go2rtc_process.c +++ b/src/video/go2rtc/go2rtc_process.c @@ -21,6 +21,7 @@ #include "video/go2rtc/go2rtc_api.h" #include "core/logger.h" #include "core/config.h" +#include "core/path_utils.h" #include "utils/strings.h" @@ -484,12 +485,9 @@ bool go2rtc_process_init(const char *binary_path, const char *config_dir, int ap } // Check if config directory exists, create if not - struct stat st = {0}; - if (stat(config_dir, &st) == -1) { - if (mkdir(config_dir, 0755) == -1) { - log_error("Failed to create go2rtc config directory: %s", config_dir); - return false; - } + if (mkdir_recursive(config_dir)) { + log_error("Failed to create go2rtc config directory: %s", config_dir); + return false; } // Store config directory diff --git a/src/video/hls/hls_directory.c b/src/video/hls/hls_directory.c index 10bfaf97..88944969 100644 --- a/src/video/hls/hls_directory.c +++ b/src/video/hls/hls_directory.c @@ -67,7 +67,7 @@ int ensure_hls_directory(const char *output_dir, const char *stream_name) { for (char *p = temp_path + 1; *p; p++) { if (*p == '/') { *p = '\0'; - if (mkdir(temp_path, 0777) != 0 && errno != EEXIST) { + if (ensure_dir(temp_path)) { log_warn("Failed to create parent directory: %s (error: %s)", temp_path, strerror(errno)); } *p = '/'; @@ -75,7 +75,7 @@ int ensure_hls_directory(const char *output_dir, const char *stream_name) { } // Create the final directory - if (mkdir(temp_path, 0777) != 0 && errno != EEXIST) { + if (ensure_dir(temp_path)) { log_error("Failed to create output directory: %s (error: %s)", temp_path, strerror(errno)); return -1; } @@ -140,7 +140,7 @@ int ensure_hls_directory(const char *output_dir, const char *stream_name) { for (char *p = temp_path + 1; *p; p++) { if (*p == '/') { *p = '\0'; - if (mkdir(temp_path, 0755) != 0 && errno != EEXIST) { + if (ensure_dir(temp_path)) { log_warn("Failed to create parent directory: %s (error: %s)", temp_path, strerror(errno)); } *p = '/'; @@ -148,7 +148,7 @@ int ensure_hls_directory(const char *output_dir, const char *stream_name) { } // Create the final directory - if (mkdir(temp_path, 0755) != 0 && errno != EEXIST) { + if (ensure_dir(temp_path)) { log_warn("Failed to create parent directory: %s (error: %s)", temp_path, strerror(errno)); } @@ -172,7 +172,7 @@ int ensure_hls_directory(const char *output_dir, const char *stream_name) { for (char *p = retry_path + 1; *p; p++) { if (*p == '/') { *p = '\0'; - if (mkdir(retry_path, 0755) != 0 && errno != EEXIST) { + if (ensure_dir(retry_path)) { log_warn("Failed to create parent directory: %s (error: %s)", retry_path, strerror(errno)); } else { // Set permissions (owner rwx, group/other rx) @@ -183,7 +183,7 @@ int ensure_hls_directory(const char *output_dir, const char *stream_name) { } // Create the final directory - if (mkdir(retry_path, 0755) != 0 && errno != EEXIST) { + if (ensure_dir(retry_path)) { log_warn("Failed to create parent directory: %s (error: %s)", retry_path, strerror(errno)); } diff --git a/src/video/hls/hls_unified_thread.c b/src/video/hls/hls_unified_thread.c index 1a0fb911..dc029033 100644 --- a/src/video/hls/hls_unified_thread.c +++ b/src/video/hls/hls_unified_thread.c @@ -2448,7 +2448,7 @@ int start_hls_unified_stream(const char *stream_name) { for (char *p = temp_path + 1; *p; p++) { if (*p == '/') { *p = '\0'; - if (mkdir(temp_path, 0777) != 0 && errno != EEXIST) { + if (ensure_dir(temp_path)) { log_warn("Failed to create parent directory: %s (error: %s)", temp_path, strerror(errno)); } *p = '/'; @@ -2456,7 +2456,7 @@ int start_hls_unified_stream(const char *stream_name) { } // Create the final directory - if (mkdir(temp_path, 0777) != 0 && errno != EEXIST) { + if (ensure_dir(temp_path)) { log_error("Failed to create output directory: %s (error: %s)", temp_path, strerror(errno)); hls_guarded_free(ctx); return -1; @@ -2481,7 +2481,7 @@ int start_hls_unified_stream(const char *stream_name) { // Create parent directory if it doesn't exist if (stat(parent_dir, &st) != 0 || !S_ISDIR(st.st_mode)) { - if (mkdir(parent_dir, 0777) != 0 && errno != EEXIST) { + if (ensure_dir(parent_dir)) { log_warn("Failed to create parent HLS directory: %s (error: %s)", parent_dir, strerror(errno)); } } diff --git a/src/video/hls_writer.c b/src/video/hls_writer.c index dc17daa1..a50b4325 100644 --- a/src/video/hls_writer.c +++ b/src/video/hls_writer.c @@ -19,6 +19,7 @@ #include #include "core/logger.h" +#include "core/path_utils.h" #include "utils/strings.h" #include "video/hls_writer.h" #include "video/detection_integration.h" @@ -411,76 +412,16 @@ static int ensure_output_directory(hls_writer_t *writer) { safe_strcpy(writer->output_dir, safe_dir_path, MAX_PATH_LENGTH, 0); } - // Always use the safe path - dir_path = safe_dir_path; - - // Check if directory exists - if (stat(dir_path, &st) != 0 || !S_ISDIR(st.st_mode)) { - log_warn("HLS output directory does not exist or is not a directory: %s", dir_path); - - // Create directory using direct C functions to handle paths with spaces - char temp_path[MAX_PATH_LENGTH]; - safe_strcpy(temp_path, dir_path, MAX_PATH_LENGTH, 0); - - // Create parent directories one by one - for (char *p = temp_path + 1; *p; p++) { - if (*p == '/') { - *p = '\0'; - if (mkdir(temp_path, 0755) != 0 && errno != EEXIST) { - log_warn("Failed to create parent directory: %s (error: %s)", - temp_path, strerror(errno)); - } - *p = '/'; - } - } - - // Create the final directory - if (mkdir(temp_path, 0755) != 0 && errno != EEXIST) { - log_error("Failed to create output directory: %s (error: %s)", - temp_path, strerror(errno)); - return -1; - } - - log_info("Created HLS output directory: %s", dir_path); - - // Set permissions via fd to avoid TOCTOU between mkdir and chmod - { - int dir_fd = open(dir_path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); - if (dir_fd >= 0) { - if (fchmod(dir_fd, 0755) != 0) { - log_warn("Failed to set permissions on directory: %s (error: %s)", - dir_path, strerror(errno)); - } - close(dir_fd); - } - } + // Create directory if necessary + if (mkdir_recursive(safe_dir_path)) { + log_error("Failed to create HLS output directory"); + return -1; } - // Ensure the directory is writable. Use open()+fstatat()+fchmod() to avoid - // TOCTOU race between access() check and subsequent open() (#34). - { - int dir_fd = open(dir_path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); - if (dir_fd < 0) { - log_error("Failed to open HLS output directory: %s (error: %s)", dir_path, strerror(errno)); - return -1; - } - - struct stat dir_st; - if (fstat(dir_fd, &dir_st) != 0) { - log_error("Failed to stat HLS output directory fd: %s (error: %s)", dir_path, strerror(errno)); - close(dir_fd); - return -1; - } - - if (!(dir_st.st_mode & S_IWUSR)) { - log_warn("HLS output directory may not be writable: %s, attempting permission fix", dir_path); - if (fchmod(dir_fd, 0755) != 0) { - log_warn("Failed to set permissions on directory: %s (error: %s)", - dir_path, strerror(errno)); - } - } - - close(dir_fd); + // Ensure the directory is writable + if (chmod_path(safe_dir_path, 0755)) { + // Not fatal + log_warn("Failed to set permissions on directory: %s", safe_dir_path); } return 0; diff --git a/src/video/mp4_writer_utils.c b/src/video/mp4_writer_utils.c index 12db2099..f6f3e806 100644 --- a/src/video/mp4_writer_utils.c +++ b/src/video/mp4_writer_utils.c @@ -33,6 +33,7 @@ #include "core/config.h" #include "core/logger.h" +#include "core/path_utils.h" #include "utils/strings.h" #include "video/mp4_writer.h" #include "video/mp4_writer_internal.h" @@ -1016,7 +1017,7 @@ int mp4_writer_initialize(mp4_writer_t *writer, const AVPacket *pkt, const AVStr } // Set permissions to ensure it's writable - if (chmod_recursive(dir_path, 0777) != 0) { + if (chmod_path(dir_path, 0755)) { log_warn("Failed to set permissions: %s", dir_path); } diff --git a/src/video/onvif_motion_recording.c b/src/video/onvif_motion_recording.c index 6b8183be..22f46bc8 100644 --- a/src/video/onvif_motion_recording.c +++ b/src/video/onvif_motion_recording.c @@ -276,17 +276,10 @@ static int generate_recording_path(const char *stream_name, char *path, size_t p config->storage_path, sanitized_name, year, month, day); // Create directories if they don't exist - char temp_path[MAX_PATH_LENGTH]; - snprintf(temp_path, sizeof(temp_path), "%s/%s", config->storage_path, sanitized_name); - mkdir(temp_path, 0755); - - snprintf(temp_path, sizeof(temp_path), "%s/%s/%s", config->storage_path, sanitized_name, year); - mkdir(temp_path, 0755); - - snprintf(temp_path, sizeof(temp_path), "%s/%s/%s/%s", config->storage_path, sanitized_name, year, month); - mkdir(temp_path, 0755); - - mkdir(dir_path, 0755); + if (mkdir_recursive(dir_path)) { + log_error("Could not create recording directory: %s", dir_path); + return -1; + } // Generate full file path snprintf(path, path_size, "%s/%s_%s_motion.mp4", dir_path, sanitized_name, timestamp); diff --git a/src/video/packet_buffer.c b/src/video/packet_buffer.c index a2eff3da..e0abff50 100644 --- a/src/video/packet_buffer.c +++ b/src/video/packet_buffer.c @@ -16,6 +16,7 @@ #include "video/packet_buffer.h" #include "core/logger.h" #include "core/config.h" +#include "core/path_utils.h" #include "utils/strings.h" // Global buffer pool @@ -264,7 +265,7 @@ 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); - mkdir(buffer->disk_buffer_path, 0755); + ensure_dir(buffer->disk_buffer_path); } } diff --git a/src/video/unified_detection_thread.c b/src/video/unified_detection_thread.c index e2168a91..0dfd2472 100644 --- a/src/video/unified_detection_thread.c +++ b/src/video/unified_detection_thread.c @@ -507,7 +507,7 @@ 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); - mkdir(ctx->output_dir, 0755); + ensure_dir(ctx->output_dir); } // If using built-in motion detection, enable the motion stream now so that @@ -1543,7 +1543,7 @@ static int udt_start_recording(unified_detection_ctx_t *ctx) { // Ensure output directory exists struct stat st = {0}; if (stat(ctx->output_dir, &st) == -1) { - if (mkdir(ctx->output_dir, 0755) != 0) { + if (ensure_dir(ctx->output_dir)) { log_error("[%s] Failed to create output directory: %s", ctx->stream_name, ctx->output_dir); return -1; } diff --git a/src/web/api_handlers_recordings_thumbnail.c b/src/web/api_handlers_recordings_thumbnail.c index b71416eb..cb023b5f 100644 --- a/src/web/api_handlers_recordings_thumbnail.c +++ b/src/web/api_handlers_recordings_thumbnail.c @@ -28,6 +28,7 @@ #define LOG_COMPONENT "RecordingsAPI" #include "core/logger.h" #include "core/config.h" +#include "core/path_utils.h" #include "database/database_manager.h" #include "database/db_recordings.h" @@ -45,7 +46,7 @@ static int ensure_thumbnails_dir(const char *storage_path) { return 0; // Already exists } - if (mkdir(dir_path, 0755) != 0 && errno != EEXIST) { + if (ensure_dir(dir_path)) { log_error("Failed to create thumbnails directory: %s (error: %s)", dir_path, strerror(errno)); return -1; diff --git a/src/web/api_handlers_system.c b/src/web/api_handlers_system.c index d3291892..41b8893c 100644 --- a/src/web/api_handlers_system.c +++ b/src/web/api_handlers_system.c @@ -38,6 +38,7 @@ #define LOG_COMPONENT "SystemAPI" #include "core/logger.h" #include "core/config.h" +#include "core/path_utils.h" #include "core/version.h" #include "core/shutdown_coordinator.h" #include "utils/strings.h" @@ -1441,7 +1442,7 @@ 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 - mkdir(backup_path, 0755); + ensure_dir(backup_path); // Append filename to path snprintf(backup_path, sizeof(backup_path), "%s/backups/%s", g_config.web_root, backup_filename); diff --git a/src/web/api_handlers_timeline.c b/src/web/api_handlers_timeline.c index 599173e2..cab4a683 100644 --- a/src/web/api_handlers_timeline.c +++ b/src/web/api_handlers_timeline.c @@ -25,6 +25,7 @@ #define LOG_COMPONENT "RecordingsAPI" #include "core/logger.h" #include "core/config.h" +#include "core/path_utils.h" #include "core/url_utils.h" #include "utils/strings.h" #include "database/database_manager.h" @@ -586,10 +587,7 @@ 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 - struct stat st = {0}; - if (stat(temp_dir, &st) == -1) { - mkdir(temp_dir, 0755); - } + ensure_dir(temp_dir); // Generate a unique manifest filename char manifest_filename[MAX_PATH_LENGTH]; diff --git a/tests/unit/test_api_handlers_system.c b/tests/unit/test_api_handlers_system.c index 35e63787..76fd4ed3 100644 --- a/tests/unit/test_api_handlers_system.c +++ b/tests/unit/test_api_handlers_system.c @@ -18,6 +18,7 @@ #include "unity.h" #include "core/config.h" #include "core/logger.h" +#include "core/path_utils.h" #include "utils/strings.h" #include "database/db_core.h" #include "database/db_streams.h" @@ -227,8 +228,7 @@ int main(void) { snprintf(g_db_path, sizeof(g_db_path), "%s/lightnvr.db", g_tmp_root); snprintf(g_storage_path, sizeof(g_storage_path), "%s/storage", g_tmp_root); - mkdir(g_tmp_root, 0755); - mkdir(g_storage_path, 0755); + mkdir_recursive(g_storage_path); safe_strcpy(g_config.storage_path, g_storage_path, sizeof(g_config.storage_path), 0); if (init_database(g_db_path) != 0) { diff --git a/tests/unit/test_config.c b/tests/unit/test_config.c index c60d8e37..eb66845e 100644 --- a/tests/unit/test_config.c +++ b/tests/unit/test_config.c @@ -20,6 +20,7 @@ #include "unity.h" #include "core/config.h" #include "core/logger.h" +#include "core/path_utils.h" /* File-scope config used by most tests — setUp zeros it, tearDown frees the dynamically allocated streams array to keep ASan leak-free. */ @@ -389,7 +390,7 @@ void test_env_integer_whitespace_handling(void) { models_path, db_path, web_root); fclose(config_file); - TEST_ASSERT_EQUAL_INT(0, mkdir(web_root, 0755)); + TEST_ASSERT_EQUAL_INT(0, ensure_dir(web_root)); const char *previous_env = getenv("LIGHTNVR_WEB_PORT"); char *saved_env = previous_env ? strdup(previous_env) : NULL; diff --git a/tests/unit/test_storage_manager_retention.c b/tests/unit/test_storage_manager_retention.c index 92ed8d8a..ac24ad65 100644 --- a/tests/unit/test_storage_manager_retention.c +++ b/tests/unit/test_storage_manager_retention.c @@ -14,6 +14,7 @@ #include #include "unity.h" +#include "core/path_utils.h" #include "database/db_core.h" #include "database/db_recordings.h" #include "database/db_streams.h" @@ -60,7 +61,7 @@ static void create_mp4_dir(void) { TEST_ASSERT_TRUE(root_len + 5 <= sizeof(dir)); memcpy(dir, g_storage_root, root_len); memcpy(dir + root_len, "/mp4", 5); - TEST_ASSERT_TRUE(mkdir(dir, 0755) == 0 || access(dir, F_OK) == 0); + TEST_ASSERT_EQUAL_INT(0, ensure_dir(dir)); } static void create_file(const char *path, size_t size) { @@ -181,7 +182,7 @@ void test_apply_retention_policy_preserves_metadata_when_file_delete_fails(void) create_mp4_dir(); add_stream_with_quota("blocked_cam", 1); mp4_path(blocked_path, sizeof(blocked_path), "quota-blocked.mp4"); - TEST_ASSERT_EQUAL_INT(0, mkdir(blocked_path, 0755)); + TEST_ASSERT_EQUAL_INT(0, ensure_dir(blocked_path)); rec = make_recording("blocked_cam", blocked_path, now - 300, 2 * 1024 * 1024); TEST_ASSERT_NOT_EQUAL(0, add_recording_metadata(&rec)); From 1afc16e43b3e2dacbfad7595e3b79b11d2c9a495 Mon Sep 17 00:00:00 2001 From: Dane Wagner Date: Mon, 13 Apr 2026 14:44:07 -0500 Subject: [PATCH 3/3] Review comments --- include/core/path_utils.h | 5 +++-- src/core/daemon.c | 1 + src/core/path_utils.c | 27 +++++++++++++++++---------- src/video/hls_writer.c | 2 +- src/video/packet_buffer.c | 8 +++++++- src/video/unified_detection_thread.c | 7 ++++++- src/web/api_handlers_system.c | 6 +++++- src/web/api_handlers_timeline.c | 5 ++++- 8 files changed, 44 insertions(+), 17 deletions(-) diff --git a/include/core/path_utils.h b/include/core/path_utils.h index e7a47c96..4448e81c 100644 --- a/include/core/path_utils.h +++ b/include/core/path_utils.h @@ -2,7 +2,7 @@ #define LIGHTNVR_PATH_UTILS_H #include -#include +#include /** * 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); diff --git a/src/core/daemon.c b/src/core/daemon.c index 6ac6004c..0db378cc 100644 --- a/src/core/daemon.c +++ b/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; } diff --git a/src/core/path_utils.c b/src/core/path_utils.c index bd9a403d..7bf6ff6e 100644 --- a/src/core/path_utils.c +++ b/src/core/path_utils.c @@ -1,14 +1,12 @@ #include #include #include +#include #include +#include #include #include -#include -#include -#include #include -#include #include #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) { diff --git a/src/video/hls_writer.c b/src/video/hls_writer.c index a50b4325..123e097f 100644 --- a/src/video/hls_writer.c +++ b/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; } diff --git a/src/video/packet_buffer.c b/src/video/packet_buffer.c index e0abff50..390644ab 100644 --- a/src/video/packet_buffer.c +++ b/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; + } } } diff --git a/src/video/unified_detection_thread.c b/src/video/unified_detection_thread.c index 0dfd2472..1728baa1 100644 --- a/src/video/unified_detection_thread.c +++ b/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 diff --git a/src/web/api_handlers_system.c b/src/web/api_handlers_system.c index 41b8893c..3feb31ed 100644 --- a/src/web/api_handlers_system.c +++ b/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); diff --git a/src/web/api_handlers_timeline.c b/src/web/api_handlers_timeline.c index cab4a683..4b593632 100644 --- a/src/web/api_handlers_timeline.c +++ b/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];