Skip to content

Fix memory safety, misleading error messages, logging correctness, and React hook bugs#184

Merged
matteius merged 2 commits intomainfrom
copilot/fix-g-config-dir-null
Mar 6, 2026
Merged

Fix memory safety, misleading error messages, logging correctness, and React hook bugs#184
matteius merged 2 commits intomainfrom
copilot/fix-g-config-dir-null

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 6, 2026

Applies a collection of targeted correctness fixes across the go2rtc process management, HLS thread, MP4/audio transcoding, ONVIF auth, and WebRTC UI layers.

go2rtc_process.c

  • Null g_config_dir after free on malloc failure — prevents use-after-free if the function is called again after partial init
  • Distinct error messages for open() vs fdopen() failures — previously both printed the same string, making log triage ambiguous
  • PATH_MAX instead of 1024 for log_path/log_dir stack buffers — 1024 is arbitrary and smaller than PATH_MAX (4096) defined in the same file
  • fprintf(stderr, ...) in child process post-fork() — calling the logging system before exec() in the child is unsafe due to shared file descriptors

hls_unified_thread.c

  • Remove is_stream_stopping(NULL) from the av_freep guard — passing NULL has undefined behavior; is_shutdown_initiated() alone is the correct condition here
  • Remove date from CRITICAL FIX comment — the hardcoded 2025-04-11 was misleading (current date is 2026)

mp4_writer_utils.c / mp4_writer_internal.h

  • Rename transcode_mulaw_to_aactranscode_pcm_to_aac — the function handles μ-law, A-law, S16LE, and other PCM variants; the old name misrepresented its scope. Updated definition, header declaration, and all call sites.
  • Fix log/error strings — "μ-law to AAC" and "μ-law decoder" replaced with "PCM to AAC" and "PCM decoder"

onvif_device_management.c

  • Fix WS-Security UsernameToken digestpassword was copied with password_len + 1 (including null terminator); the spec requires raw bytes only:
    // Before
    memcpy(... + nonce_len + created_len, password, password_len + 1);
    // After
    memcpy(... + nonce_len + created_len, password, password_len);

WebRTCView.jsx

  • Break selectedStreamsetSelectedStream infinite loop — added early return if (selectedStream) return; so the stream-processing effect never overrides an already-selected stream
  • Fix stale closure / exhaustive-deps — dependency array changed from [streamsData, selectedStream] to [streamsData, autoGrid]; autoGrid was read inside the effect but omitted from deps
Original prompt
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"If malloc fails at line 404, g_config_dir is freed but the function returns false without setting g_initialized, meaning subsequent calls could still attempt to use the freed g_config_dir. Consider setting g_config_dir to NULL after freeing it, or setting it to NULL at initialization to ensure a safe state.","fixFiles":[{"filePath":"src/video/go2rtc/go2rtc_process.c","diff":"diff --git a/src/video/go2rtc/go2rtc_process.c b/src/video/go2rtc/go2rtc_process.c\n--- a/src/video/go2rtc/go2rtc_process.c\n+++ b/src/video/go2rtc/go2rtc_process.c\n@@ -405,6 +405,7 @@\n     if (!g_config_path) {\n         log_error(\"Memory allocation failed for config path\");\n         free(g_config_dir);\n+        g_config_dir = NULL;\n         return false;\n     }\n \n"}]},{"message":"Lines 471 and 476 have identical error messages, making it difficult to distinguish between open() failure and fdopen() failure during debugging. The second error message should clarify that fdopen() failed, for example: 'Failed to create file stream for go2rtc config file'.","fixFiles":[{"filePath":"src/video/go2rtc/go2rtc_process.c","diff":"diff --git a/src/video/go2rtc/go2rtc_process.c b/src/video/go2rtc/go2rtc_process.c\n--- a/src/video/go2rtc/go2rtc_process.c\n+++ b/src/video/go2rtc/go2rtc_process.c\n@@ -473,7 +473,7 @@\n     }\n     FILE *config_file = fdopen(config_fd, \"w\");\n     if (!config_file) {\n-        log_error(\"Failed to open go2rtc config file for writing: %s\", config_path);\n+        log_error(\"Failed to create file stream for go2rtc config file: %s\", config_path);\n         close(config_fd);\n         return false;\n     }\n"}]},{"message":"The comment indicates the array size is chosen instead of PATH_MAX, but 1024 is arbitrary and may be insufficient on some systems where PATH_MAX is larger (e.g., 4096 as defined on line 28). Consider using PATH_MAX consistently or documenting why 1024 is sufficient for this use case.","fixFiles":[{"filePath":"src/video/go2rtc/go2rtc_process.c","diff":"diff --git a/src/video/go2rtc/go2rtc_process.c b/src/video/go2rtc/go2rtc_process.c\n--- a/src/video/go2rtc/go2rtc_process.c\n+++ b/src/video/go2rtc/go2rtc_process.c\n@@ -1137,10 +1137,10 @@\n         }\n \n         // Redirect stdout and stderr to log files\n-        char log_path[1024]; // Use a reasonable fixed size instead of PATH_MAX\n+        char log_path[PATH_MAX]; // Use PATH_MAX to accommodate full filesystem paths\n \n         // Extract directory from g_config->log_file\n-        char log_dir[1024] = {0};\n+        char log_dir[PATH_MAX] = {0};\n         if (g_config.log_file[0] != '\\0') {\n             strncpy(log_dir, g_config.log_file, sizeof(log_dir) - 1);\n \n"}]},{"message":"The size of log_dir (1024) is arbitrary and may not match the actual size of g_config.log_file. If g_config.log_file is larger than 1023 characters, strncpy will truncate silently, potentially causing incorrect paths. Consider using PATH_MAX or documenting the assumption about g_config.log_file's maximum length.","fixFiles":[{"filePath":"src/video/go2rtc/go2rtc_process.c","diff":"diff --git a/src/video/go2rtc/go2rtc_process.c b/src/video/go2rtc/go2rtc_process.c\n--- a/src/video/go2rtc/go2rtc_process.c\n+++ b/src/video/go2rtc/go2rtc_process.c\n@@ -1137,10 +1137,10 @@\n         }\n \n         // Redirect stdout and stderr to log files\n-        char log_path[1024]; // Use a reasonable fixed size instead of PATH_MAX\n+        char log_path[PATH_MAX];\n \n         // Extract directory from g_config->log_file\n-        char log_dir[1024] = {0};\n+        char log_dir[PATH_MAX] = {0};\n         if (g_config.log_file[0] != '\\0') {\n             strncpy(log_dir, g_config.log_file, sizeof(log_dir) - 1);\n \n"}]},{"message":"The log_info call at line 1164 occurs in the child process after fork(), but the logging system may not be properly configured for the child process (it might share file descriptors with the parent). Logging from the child before exec can cause issues. Consider using fprintf(stderr, ...) instead of log_info/log_error in the child process before exec.","fixFiles":[{"filePath":"src/video/go2rtc/go2rtc_process.c","diff":"diff --git a/src/video/go2rtc/go2rtc_process.c b/src/video/go2rtc/go2rtc_process.c\n--- a/src/video/go2rtc/go2rtc_process.c\n+++ b/src/video/go2rtc/go2rtc_process.c\n@@ -1161,11 +1161,11 @@\n         }\n \n         // Log the path we're using for the log file\n-        log_info(\"Using go2rtc log file: %s\", log_path);\n+        fprintf(stderr, \"Using go2rtc log file: %s\\n\", log_path);\n \n         int log_fd = open(log_path, O_WRONLY | O_CREAT | O_APPEND, 0644);\n         if (log_fd == -1) {\n-            log_error(\"Failed to open log file: %s\", log_path);\n+            fprintf(stderr, \"Failed to open log file: %s\\n\", log_path);\n             ...

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…oss multiple files

Co-authored-by: matteius <479892+matteius@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix memory handling for g_config_dir in go2rtc_process Fix memory safety, misleading error messages, logging correctness, and React hook bugs Mar 6, 2026
@matteius matteius marked this pull request as ready for review March 6, 2026 23:53
@matteius matteius merged commit 5244ff3 into main Mar 6, 2026
2 checks passed
@matteius matteius deleted the copilot/fix-g-config-dir-null branch March 6, 2026 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants