Skip to content

Commit 69beb90

Browse files
sunilnomroshan-kudmkarthi
authored
Fix thread safety, input validation, and error handling (#6)
* Fix thread safety, input validation, and error handling - session_manager.h: make frame_counter atomic for thread safety - main.c: add --test-time CLI option for timed transmit runs - config_reader.c: replace atoi with strtol, fix null-app check, handle PCI BDF regex compilation failure - logger.c: guard level check with mutex, handle freopen failure - test_config_reader.c: update null-app test to expect error return * Fix cppcheck and shellcheck warnings - mtl_tx.c: move null check before pointer arithmetic to fix nullPointerArithmeticRedundantCheck - logger.c: assign freopen result back to file_fp to fix resourceLeak - build.sh, test.sh: remove UTF-8 BOM (SC1082) --------- Co-authored-by: roshan-ku <roshan.kumar@intel.com> Co-authored-by: D M, Karthik <karthik.d.m@intel.com>
1 parent aa9ff84 commit 69beb90

8 files changed

Lines changed: 61 additions & 29 deletions

File tree

include/core/session_manager.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#pragma once
66

7+
#include <stdatomic.h>
78
#include <stdint.h>
89
#include <stdbool.h>
910
#include <stdio.h>
@@ -57,7 +58,7 @@ struct shared_decode_ctx {
5758
bool start_ready;
5859

5960
struct dvledtx_context* app;
60-
uint32_t frame_counter; /* shared monotonic frame number */
61+
_Atomic uint32_t frame_counter; /* shared monotonic frame number */
6162
};
6263

6364
/* Video TX session context */

scripts/build.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#!/bin/bash
1+
#!/bin/bash
22
# SPDX-License-Identifier: BSD-3-Clause
33
# Copyright 2026 Intel Corporation
44

scripts/test.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#!/bin/bash
1+
#!/bin/bash
22
# SPDX-License-Identifier: BSD-3-Clause
33
# Copyright 2026 Intel Corporation
44

src/main.c

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -154,30 +154,42 @@ static bool validate_log_path(const char* path) {
154154
}
155155

156156
static void print_help(const char* prog_name) {
157-
LOG_INFO("Usage: %s --config <file>", prog_name);
157+
LOG_INFO("Usage: %s --config <file> [options]", prog_name);
158158
LOG_INFO("Options:");
159-
LOG_INFO(" -C, --config <file> JSON config file (required)");
160-
LOG_INFO(" -v, --version Show version");
161-
LOG_INFO(" --help Show this help");
159+
LOG_INFO(" -C, --config <file> JSON config file (required)");
160+
LOG_INFO(" -t, --test-time <secs> Transmit for N seconds then exit (1..86400)");
161+
LOG_INFO(" -v, --version Show version");
162+
LOG_INFO(" --help Show this help");
162163
}
163164

164165
static int parse_args(struct dvledtx_context* ctx, int argc, char** argv) {
165166
static struct option long_options[] = {
166-
{"config", required_argument, 0, 'C'},
167-
{"version", no_argument, 0, 'v'},
168-
{"help", no_argument, 0, '?'},
167+
{"config", required_argument, 0, 'C'},
168+
{"test-time", required_argument, 0, 't'},
169+
{"version", no_argument, 0, 'v'},
170+
{"help", no_argument, 0, '?'},
169171
{0, 0, 0, 0}
170172
};
171173

172174
ctx->config_file[0] = '\0';
173175

174176
int c = 0, option_index = 0;
175-
while ((c = getopt_long(argc, argv, "C:v?", long_options, &option_index)) != -1) {
177+
while ((c = getopt_long(argc, argv, "C:t:v?", long_options, &option_index)) != -1) { /* flawfinder: ignore */
176178
switch (c) {
177179
case 'C':
178180
strncpy(ctx->config_file, optarg, sizeof(ctx->config_file) - 1);
179181
ctx->config_file[sizeof(ctx->config_file) - 1] = '\0';
180182
break;
183+
case 't': {
184+
char *endptr = NULL;
185+
long val = strtol(optarg, &endptr, 10);
186+
if (endptr == optarg || *endptr != '\0' || val <= 0 || val > 86400) {
187+
LOG_ERROR("Invalid --test-time value '%s' (expected 1..86400)", optarg);
188+
return -1;
189+
}
190+
ctx->test_time_s = (int)val;
191+
break;
192+
}
181193
case 'v':
182194
printf("dvledtx version %s\n", DVLEDTX_VERSION);
183195
exit(0);

src/mtl/mtl_tx.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,13 +145,13 @@ void mtl_copy_crop_to_frame(struct st_frame* dst, AVFrame* src,
145145
* Y/U/V planes are packed sequentially: Y then U then V, each with
146146
* tightly-packed rows (no line padding). Compute the offsets manually. */
147147
uint8_t* dst_y = (uint8_t*)dst->addr[0];
148+
if (!dst_y) return;
149+
148150
uint8_t* dst_u = dst->addr[1] ? (uint8_t*)dst->addr[1]
149151
: dst_y + dst_y_stride * crop_h;
150152
uint8_t* dst_v = dst->addr[2] ? (uint8_t*)dst->addr[2]
151153
: dst_u + dst_c_stride * chroma_h;
152154

153-
if (!dst_y) return;
154-
155155
/* Luma (Y) plane — full crop_h rows, crop_w samples wide */
156156
for (int line = 0; line < crop_h; line++)
157157
memcpy(dst_y + line * dst_y_stride,

src/util/config_reader.c

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
#include "util/config_reader.h"
77
#include "app_context.h"
88
#include "util/logger.h"
9+
#include <errno.h>
10+
#include <limits.h>
911
#include <stdio.h>
1012
#include <stdlib.h>
1113
#include <string.h>
@@ -94,7 +96,12 @@ static int extract_json_int(const char* start, const char* end, const char* key)
9496
if (pos >= end || *pos != ':') continue;
9597
pos++;
9698
while (pos < end && (*pos == ' ' || *pos == '\t' || *pos == '\n' || *pos == '\r')) pos++;
97-
return atoi(pos);
99+
char *endptr = NULL;
100+
errno = 0;
101+
long val = strtol(pos, &endptr, 10);
102+
if (endptr == pos || errno == ERANGE || val < INT_MIN || val > INT_MAX)
103+
return -1;
104+
return (int)val;
98105
}
99106
return -1;
100107
}
@@ -391,15 +398,17 @@ int validate_tx_config(const struct dvledtx_config* config) {
391398
int reti = regcomp(&regex,
392399
"^[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}\\.[0-9]$",
393400
REG_EXTENDED | REG_NOSUB);
394-
if (reti == 0) {
395-
reti = regexec(&regex, config->interface_name, 0, NULL, 0);
396-
regfree(&regex);
397-
if (reti != 0) {
398-
LOG_ERROR("Invalid PCI BDF format '%s' "
399-
"(expected DDDD:DD:DD.D hex pattern)",
400-
config->interface_name);
401-
return -1;
402-
}
401+
if (reti != 0) {
402+
LOG_ERROR("Internal error: PCI BDF regex compilation failed");
403+
return -1;
404+
}
405+
reti = regexec(&regex, config->interface_name, 0, NULL, 0);
406+
regfree(&regex);
407+
if (reti != 0) {
408+
LOG_ERROR("Invalid PCI BDF format '%s' "
409+
"(expected DDDD:DD:DD.D hex pattern)",
410+
config->interface_name);
411+
return -1;
403412
}
404413
}
405414

@@ -546,7 +555,9 @@ int validate_tx_config(const struct dvledtx_config* config) {
546555
}
547556

548557
int load_and_apply_config(struct dvledtx_context* app, const char* config_file) {
549-
if (app == NULL || config_file == NULL || config_file[0] == '\0')
558+
if (app == NULL)
559+
return -1;
560+
if (config_file == NULL || config_file[0] == '\0')
550561
return 0; /* no config file — keep CLI defaults */
551562

552563
struct dvledtx_config config;

src/util/logger.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ log_level_t logger_get_level(void)
103103

104104
bool logger_is_level_enabled(log_level_t level)
105105
{
106-
return g_logger.initialized && (level <= g_logger.config.level);
106+
pthread_mutex_lock(&g_logger.lock);
107+
bool enabled = g_logger.initialized && (level <= g_logger.config.level);
108+
pthread_mutex_unlock(&g_logger.lock);
109+
return enabled;
107110
}
108111

109112
void logger_log(log_level_t level, const char *file, int line,
@@ -156,7 +159,12 @@ void logger_log(log_level_t level, const char *file, int line,
156159
if (g_logger.config.enable_file && g_logger.file_fp) {
157160
/* If the file has reached 20 MB, truncate and start fresh */
158161
if (ftell(g_logger.file_fp) >= 20 * 1024 * 1024) {
159-
freopen(g_logger.config.log_file, "w", g_logger.file_fp);
162+
FILE *new_fp = freopen(g_logger.config.log_file, "w", g_logger.file_fp);
163+
if (new_fp == NULL) {
164+
g_logger.file_fp = NULL;
165+
} else {
166+
g_logger.file_fp = new_fp;
167+
}
160168
}
161169
if (level == LOG_LEVEL_ERROR) {
162170
fprintf(g_logger.file_fp, "%s[%s:%d %s] %s\n",

tests/test_config_reader.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -956,10 +956,10 @@ static void test_parse_session_zero_crop_w_fails(void **state)
956956
* load_and_apply_config
957957
* ========================================================================== */
958958

959-
static void test_load_and_apply_config_null_app_returns_zero(void **state)
959+
static void test_load_and_apply_config_null_app_returns_error(void **state)
960960
{
961961
(void)state;
962-
assert_int_equal(load_and_apply_config(NULL, "any_file.json"), 0);
962+
assert_int_equal(load_and_apply_config(NULL, "any_file.json"), -1);
963963
}
964964

965965
static void test_load_and_apply_config_null_file_returns_zero(void **state)
@@ -1175,7 +1175,7 @@ int main(void)
11751175
cmocka_unit_test(test_peek_log_file_strips_bare_control_chars),
11761176

11771177
/* --- load_and_apply_config --- */
1178-
cmocka_unit_test(test_load_and_apply_config_null_app_returns_zero),
1178+
cmocka_unit_test(test_load_and_apply_config_null_app_returns_error),
11791179
cmocka_unit_test(test_load_and_apply_config_null_file_returns_zero),
11801180
cmocka_unit_test(test_load_and_apply_config_empty_file_returns_zero),
11811181
cmocka_unit_test(test_load_and_apply_config_nonexistent_file_returns_minus1),

0 commit comments

Comments
 (0)