Conversation
📝 WalkthroughWalkthroughThis PR adds progress reporting capabilities to the WFP scanning functionality. It introduces a new Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as Command Line
participant Main as main.c
participant Scan as scan.c
participant FS as File System
participant StatusFile as Status File<br/>(/tmp/engine)
User->>CLI: Invoke with --report-progress
CLI->>Main: Parse arguments
Main->>Main: Extract batch ID from path
Main->>FS: Setup output redirection
Main->>Scan: Call wfp_scan(path, ..., report_progress=true)
Scan->>StatusFile: Create status file
Scan->>StatusFile: Write initial "scanning" status
loop For each 1% progress change
Scan->>Scan: Count lines, compute progress %
Scan->>StatusFile: Write JSON progress update
end
alt File open success
Scan->>Scan: Process WFP data
Scan->>StatusFile: Write "completed" with 100%
else File open failure
Scan->>StatusFile: Write "failed" status
end
Scan->>Main: Return scan results
Main->>StatusFile: Read final status/results
Main->>User: Display results or batch status
Main->>FS: Cleanup batch resources
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@src/main.c`:
- Around line 264-279: The code is vulnerable to path traversal because
user-supplied IDs passed into show_batch_result and show_batch_status are
concatenated into file paths without sanitization; add a validation helper
(e.g., is_valid_scanid) that returns false for any scan id containing '/' or
".." or any character outside [A-Za-z0-9-], and use it to reject/return an error
before calling show_batch_result, show_batch_status (and any other places that
accept raw IDs, including the -R flow where get_scanid_from_path is used) so
only validated IDs are used to build file paths.
- Around line 768-775: When scan_report_progress is true but
get_scanid_from_path(target) returns NULL, the batch redirection is silently
skipped; change the logic around batch_scanid in main.c to detect a NULL return
from get_scanid_from_path(target) and either call setup_batch_result_output with
a safe fallback ID or emit a warning via the existing logging mechanism (e.g.,
log_warn/log_error) so the user knows redirection failed; update the branch that
currently only calls setup_batch_result_output(batch_scanid) to handle the NULL
case (using batch_scanid, get_scanid_from_path, setup_batch_result_output and
the same scan progress flow in src/scan.c) and ensure behavior is consistent
with how status files are created so output doesn’t silently stay on stdout.
- Around line 286-305: setup_batch_result_output currently hijacks stdout via
freopen(result_path, "w", stdout) which causes all subsequent stdout output
(including later informational or error messages) to be written into the result
file; instead open the result file with fopen into a dedicated FILE* (e.g.,
result_fp) rather than calling freopen, update callers or make result_fp
accessible (or return it) so result output goes to that FILE*, and ensure you
fclose(result_fp) in cleanup; if you must redirect stdout, capture and restore
the original stdout file descriptor using dup/dup2 and provide a corresponding
restore function, but prefer the simpler fix: replace freopen usage in
setup_batch_result_output with fopen into a new result FILE* and adjust code
paths to write results to result_fp while leaving stdout/stderr unchanged (refer
to function setup_batch_result_output and any cleanup path that should close the
file).
- Around line 269-278: The code must check for allocation failures: after
calling strdup(path) assign to path_copy and if it's NULL, return NULL (or
propagate error) without calling basename; after strdup(base) assign to scanid
and if it's NULL, free path_copy and return NULL; only call basename and strrchr
when their inputs are non-NULL; ensure any allocated memory (path_copy) is freed
on all error paths before returning and the function returns a clear error
indicator (e.g., NULL) when strdup fails.
- Around line 402-453: clean_old_status_files currently unlinks only the status
file; update it to also remove the matching result file in RESULT_DIR when a
status file is deleted. Inside the block where unlink(filepath) succeeds (use
the same entry->d_name), build a result path (e.g., snprintf(resultpath,
sizeof(resultpath), "%s/%s", RESULT_DIR, entry->d_name)), attempt
unlink(resultpath) if it exists, and log success/failure (e.g., printf("Removed
result: %s\n", entry->d_name) or an error). Use the existing symbols filepath,
entry->d_name, RESULT_DIR, cleaned and ensure you only increment cleaned for the
cleaned status file (leave error handling and fclose/closedir as currently
implemented).
In `@src/scan.c`:
- Around line 233-243: The helper that extracts a scan id is not checking strdup
return values (path_copy and scanid), causing possible NULL dereferences in
get_scanid_from_path (use of path_copy, basename, scanid, strrchr). Fix by
checking the result of strdup for NULL after creating path_copy and again after
creating scanid; on allocation failure free any allocated resources and return
NULL (or propagate an error) instead of proceeding to basename/strrchr. Ensure
you still free path_copy when appropriate before returning.
- Around line 252-265: Replace the non-portable time_t formatting and parsing by
using intmax_t/PRIdMAX for output and strtoimax for input: in
write_progress_to_file (function write_progress_to_file) include <inttypes.h>
and change the fprintf to print (intmax_t)started with the PRIdMAX macro; in
main.c's clean_old_status_files replace the atol/strtol parsing with
strtoimax(...) to obtain an intmax_t and then cast to time_t (time_t started =
(time_t)parsed_val), adding minimal error/overflow checking as appropriate so
write and read use the same portable integer format.
🧹 Nitpick comments (4)
inc/scan.h (1)
84-85: Consider a config struct to reduce parameter bloat.
wfp_scannow has 10 parameters, andhash_scanhas 9. This is increasingly hard to read and maintain, and callers are prone to positional mistakes. Ascan_config_tstruct grouping the shared parameters would simplify both signatures and future extensions.This isn't blocking for this PR, but worth considering soon.
src/main.c (1)
264-279: Duplicated logic:get_scanid_from_pathis nearly identical toextract_scanid_from_pathinsrc/scan.c.Both functions extract the filename without extension from a path using the same
strdup→basename→strrchrpattern. Extract this into a shared utility (e.g., insrc/util.c) to avoid divergence.src/scan.c (2)
341-350: Double-pass over the WFP file adds unnecessary I/O overhead.The entire file is read once just to count lines (lines 341-349), then rewound and read again for actual scanning. For large WFP files this doubles I/O time. Since progress reporting is optional, consider either:
- Using
fstatto get file size and tracking byte offset (ftell) as a proxy for progress, or- Counting lines only when
status_enabledis true (currently it always counts).Option 1 avoids the extra pass entirely:
Sketch using byte-offset progress
- /* Count total lines first for progress calculation */ - scanlog("Counting total lines...\n"); - char *count_line = NULL; - size_t count_len = 0; - while (getline(&count_line, &count_len, fp) != -1) { - total_lines++; - } - if (count_line) free(count_line); - rewind(fp); - scanlog("Total lines: %ld\n", total_lines); + /* Get file size for progress calculation */ + long file_size_bytes = 0; + if (status_enabled) { + fseek(fp, 0, SEEK_END); + file_size_bytes = ftell(fp); + rewind(fp); + }Then in the loop, use
ftell(fp)instead ofcurrent_linefor percentage:- int new_progress = (int)((current_line * 100) / total_lines); + int new_progress = (file_size_bytes > 0) ? (int)((ftell(fp) * 100) / file_size_bytes) : 0;
267-289: Hardcoded/tmp/engine/batch-scanpath duplicated across files.This path appears as a literal here and as
STATUS_DIRinsrc/main.c. A shared constant would prevent drift.
| static char* get_scanid_from_path(const char *path) | ||
| { | ||
| if (!path) return NULL; | ||
|
|
||
| /* Get basename */ | ||
| char *path_copy = strdup(path); | ||
| char *base = basename(path_copy); | ||
|
|
||
| /* Remove extension */ | ||
| char *scanid = strdup(base); | ||
| char *dot = strrchr(scanid, '.'); | ||
| if (dot) *dot = '\0'; | ||
|
|
||
| free(path_copy); | ||
| return scanid; | ||
| } |
There was a problem hiding this comment.
Path traversal vulnerability: user-supplied scan IDs are used unsanitized in file paths.
--batch-result ID and --batch-status ID pass optarg directly to show_batch_result / show_batch_status, which concatenate it into a path like /tmp/engine/batch-scan/<ID>. A malicious user can supply ../../etc/passwd to read arbitrary files.
get_scanid_from_path (used for the -R flow) strips to a basename, but the --batch-result / --batch-status paths bypass this entirely and use raw user input.
Add input validation to reject scan IDs containing /, .., or any non-alphanumeric/non-hyphen characters.
🛡️ Proposed fix: add a validation helper
+/**
+ * `@brief` Validates a scan ID contains only safe characters
+ * `@param` scanid Scan ID to validate
+ * `@return` true if valid, false otherwise
+ */
+static bool is_valid_scanid(const char *scanid)
+{
+ if (!scanid || *scanid == '\0') return false;
+ for (const char *p = scanid; *p; p++) {
+ if (!isalnum(*p) && *p != '-' && *p != '_') return false;
+ }
+ return true;
+}Then guard usage:
case 262: /* --batch-result */
- if (show_batch_result(optarg) == 0)
+ if (!is_valid_scanid(optarg)) {
+ fprintf(stderr, "Error: Invalid scan ID\n");
+ exit(EXIT_FAILURE);
+ }
+ if (show_batch_result(optarg) == 0)
exit(EXIT_SUCCESS);Apply the same guard for case 263 (--batch-status).
Also applies to: 313-365, 372-395
🧰 Tools
🪛 Cppcheck (2.19.0)
[warning] 274-274: If memory allocation fails, then there is a possible null pointer dereference
(nullPointerOutOfMemory)
🤖 Prompt for AI Agents
In `@src/main.c` around lines 264 - 279, The code is vulnerable to path traversal
because user-supplied IDs passed into show_batch_result and show_batch_status
are concatenated into file paths without sanitization; add a validation helper
(e.g., is_valid_scanid) that returns false for any scan id containing '/' or
".." or any character outside [A-Za-z0-9-], and use it to reject/return an error
before calling show_batch_result, show_batch_status (and any other places that
accept raw IDs, including the -R flow where get_scanid_from_path is used) so
only validated IDs are used to build file paths.
| char *path_copy = strdup(path); | ||
| char *base = basename(path_copy); | ||
|
|
||
| /* Remove extension */ | ||
| char *scanid = strdup(base); | ||
| char *dot = strrchr(scanid, '.'); | ||
| if (dot) *dot = '\0'; | ||
|
|
||
| free(path_copy); | ||
| return scanid; |
There was a problem hiding this comment.
Missing NULL check after strdup (static analysis: nullPointerOutOfMemory).
If strdup(path) on line 269 returns NULL (OOM), basename(path_copy) on line 270 dereferences it. Same for strdup(base) on line 273—if it fails, strrchr(scanid, '.') on line 274 dereferences NULL.
Proposed fix
char *path_copy = strdup(path);
+ if (!path_copy) return NULL;
char *base = basename(path_copy);
/* Remove extension */
char *scanid = strdup(base);
+ if (!scanid) {
+ free(path_copy);
+ return NULL;
+ }
char *dot = strrchr(scanid, '.');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| char *path_copy = strdup(path); | |
| char *base = basename(path_copy); | |
| /* Remove extension */ | |
| char *scanid = strdup(base); | |
| char *dot = strrchr(scanid, '.'); | |
| if (dot) *dot = '\0'; | |
| free(path_copy); | |
| return scanid; | |
| char *path_copy = strdup(path); | |
| if (!path_copy) return NULL; | |
| char *base = basename(path_copy); | |
| /* Remove extension */ | |
| char *scanid = strdup(base); | |
| if (!scanid) { | |
| free(path_copy); | |
| return NULL; | |
| } | |
| char *dot = strrchr(scanid, '.'); | |
| if (dot) *dot = '\0'; | |
| free(path_copy); | |
| return scanid; |
🧰 Tools
🪛 Cppcheck (2.19.0)
[warning] 274-274: If memory allocation fails, then there is a possible null pointer dereference
(nullPointerOutOfMemory)
🤖 Prompt for AI Agents
In `@src/main.c` around lines 269 - 278, The code must check for allocation
failures: after calling strdup(path) assign to path_copy and if it's NULL,
return NULL (or propagate error) without calling basename; after strdup(base)
assign to scanid and if it's NULL, free path_copy and return NULL; only call
basename and strrchr when their inputs are non-NULL; ensure any allocated memory
(path_copy) is freed on all error paths before returning and the function
returns a clear error indicator (e.g., NULL) when strdup fails.
| static int setup_batch_result_output(const char *scanid) | ||
| { | ||
| if (!scanid) return -1; | ||
|
|
||
| /* Create directories */ | ||
| mkdir("/tmp/engine", 0755); | ||
| mkdir(RESULT_DIR, 0755); | ||
|
|
||
| /* Build result file path */ | ||
| char result_path[MAX_PATH]; | ||
| snprintf(result_path, sizeof(result_path), "%s/%s", RESULT_DIR, scanid); | ||
|
|
||
| /* Redirect stdout to file */ | ||
| if (freopen(result_path, "w", stdout) == NULL) { | ||
| fprintf(stderr, "Error: Could not redirect output to %s: %s\n", result_path, strerror(errno)); | ||
| return -1; | ||
| } | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
freopen on stdout is never restored — downstream output (e.g., cleanup messages, error paths) silently goes to the result file.
After setup_batch_result_output, all printf/fprintf(stdout, ...) calls, including potential error messages, will be written to the batch result file. If an error occurs after this point, the user sees nothing on the terminal. Consider using a separate FILE* for result output instead of hijacking stdout, or at minimum document this intentional behavior and ensure all error paths use stderr.
🤖 Prompt for AI Agents
In `@src/main.c` around lines 286 - 305, setup_batch_result_output currently
hijacks stdout via freopen(result_path, "w", stdout) which causes all subsequent
stdout output (including later informational or error messages) to be written
into the result file; instead open the result file with fopen into a dedicated
FILE* (e.g., result_fp) rather than calling freopen, update callers or make
result_fp accessible (or return it) so result output goes to that FILE*, and
ensure you fclose(result_fp) in cleanup; if you must redirect stdout, capture
and restore the original stdout file descriptor using dup/dup2 and provide a
corresponding restore function, but prefer the simpler fix: replace freopen
usage in setup_batch_result_output with fopen into a new result FILE* and adjust
code paths to write results to result_fp while leaving stdout/stderr unchanged
(refer to function setup_batch_result_output and any cleanup path that should
close the file).
| int clean_old_status_files(void) | ||
| { | ||
| DIR *dir = opendir(STATUS_DIR); | ||
| if (!dir) { | ||
| printf("Status directory does not exist: %s\n", STATUS_DIR); | ||
| return 0; | ||
| } | ||
|
|
||
| struct dirent *entry; | ||
| int cleaned = 0; | ||
| time_t now = time(NULL); | ||
| char filepath[MAX_PATH]; | ||
| char buffer[512]; | ||
|
|
||
| while ((entry = readdir(dir)) != NULL) { | ||
| if (entry->d_name[0] == '.') continue; | ||
|
|
||
| snprintf(filepath, sizeof(filepath), "%s/%s", STATUS_DIR, entry->d_name); | ||
|
|
||
| FILE *fp = fopen(filepath, "r"); | ||
| if (!fp) continue; | ||
|
|
||
| if (fgets(buffer, sizeof(buffer), fp) != NULL) { | ||
| /* Parse JSON: {"started":TIMESTAMP,"status":"STATUS","progress":NUM} */ | ||
| char *status_ptr = strstr(buffer, "\"status\":\""); | ||
| char *started_ptr = strstr(buffer, "\"started\":"); | ||
|
|
||
| if (status_ptr && started_ptr) { | ||
| /* Check if status is "completed" */ | ||
| if (strncmp(status_ptr + 10, "completed", 9) == 0) { | ||
| /* Extract timestamp */ | ||
| time_t started = atol(started_ptr + 10); | ||
|
|
||
| /* Check if older than MAX_AGE_SECONDS */ | ||
| if ((now - started) >= MAX_AGE_SECONDS) { | ||
| fclose(fp); | ||
| if (unlink(filepath) == 0) { | ||
| cleaned++; | ||
| printf("Cleaned: %s\n", entry->d_name); | ||
| } | ||
| continue; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| fclose(fp); | ||
| } | ||
|
|
||
| closedir(dir); | ||
| printf("Total files cleaned: %d\n", cleaned); | ||
| return cleaned; | ||
| } |
There was a problem hiding this comment.
clean_old_status_files also cleans corresponding result files — or does it?
When a completed status file is cleaned, the corresponding result file in RESULT_DIR is left behind. Over time, orphaned result files will accumulate in /tmp/engine/batch-result/. Consider also removing the matching result file when cleaning a status file.
Proposed addition inside the cleanup block
if (unlink(filepath) == 0) {
cleaned++;
printf("Cleaned: %s\n", entry->d_name);
+ /* Also remove the corresponding result file */
+ char result_path[MAX_PATH];
+ snprintf(result_path, sizeof(result_path), "%s/%s", RESULT_DIR, entry->d_name);
+ unlink(result_path);
}🤖 Prompt for AI Agents
In `@src/main.c` around lines 402 - 453, clean_old_status_files currently unlinks
only the status file; update it to also remove the matching result file in
RESULT_DIR when a status file is deleted. Inside the block where
unlink(filepath) succeeds (use the same entry->d_name), build a result path
(e.g., snprintf(resultpath, sizeof(resultpath), "%s/%s", RESULT_DIR,
entry->d_name)), attempt unlink(resultpath) if it exists, and log
success/failure (e.g., printf("Removed result: %s\n", entry->d_name) or an
error). Use the existing symbols filepath, entry->d_name, RESULT_DIR, cleaned
and ensure you only increment cleaned for the cleaned status file (leave error
handling and fclose/closedir as currently implemented).
| /* Redirect output to batch result file if report mode is enabled */ | ||
| char *batch_scanid = NULL; | ||
| if (scan_report_progress) { | ||
| batch_scanid = get_scanid_from_path(target); | ||
| if (batch_scanid) { | ||
| setup_batch_result_output(batch_scanid); | ||
| } | ||
| } |
There was a problem hiding this comment.
Batch output setup silently skipped if scanid extraction fails.
If get_scanid_from_path(target) returns NULL (e.g., the path has no usable filename), scan_report_progress remains true but no redirection occurs. Output goes to stdout while src/scan.c still creates status files and writes progress. Consider logging a warning or falling back gracefully.
🤖 Prompt for AI Agents
In `@src/main.c` around lines 768 - 775, When scan_report_progress is true but
get_scanid_from_path(target) returns NULL, the batch redirection is silently
skipped; change the logic around batch_scanid in main.c to detect a NULL return
from get_scanid_from_path(target) and either call setup_batch_result_output with
a safe fallback ID or emit a warning via the existing logging mechanism (e.g.,
log_warn/log_error) so the user knows redirection failed; update the branch that
currently only calls setup_batch_result_output(batch_scanid) to handle the NULL
case (using batch_scanid, get_scanid_from_path, setup_batch_result_output and
the same scan progress flow in src/scan.c) and ensure behavior is consistent
with how status files are created so output doesn’t silently stay on stdout.
| char *path_copy = strdup(path); | ||
| char *base = basename(path_copy); | ||
|
|
||
| /* Remove extension */ | ||
| char *scanid = strdup(base); | ||
| char *dot = strrchr(scanid, '.'); | ||
| if (dot) *dot = '\0'; | ||
|
|
||
| free(path_copy); | ||
| return scanid; | ||
| } |
There was a problem hiding this comment.
Missing NULL check after strdup (static analysis: nullPointerOutOfMemory).
Same issue as in src/main.c's get_scanid_from_path: if strdup fails on line 233 or 237, subsequent operations dereference NULL.
Proposed fix
char *path_copy = strdup(path);
+ if (!path_copy) return NULL;
char *base = basename(path_copy);
/* Remove extension */
char *scanid = strdup(base);
+ if (!scanid) {
+ free(path_copy);
+ return NULL;
+ }
char *dot = strrchr(scanid, '.');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| char *path_copy = strdup(path); | |
| char *base = basename(path_copy); | |
| /* Remove extension */ | |
| char *scanid = strdup(base); | |
| char *dot = strrchr(scanid, '.'); | |
| if (dot) *dot = '\0'; | |
| free(path_copy); | |
| return scanid; | |
| } | |
| char *path_copy = strdup(path); | |
| if (!path_copy) return NULL; | |
| char *base = basename(path_copy); | |
| /* Remove extension */ | |
| char *scanid = strdup(base); | |
| if (!scanid) { | |
| free(path_copy); | |
| return NULL; | |
| } | |
| char *dot = strrchr(scanid, '.'); | |
| if (dot) *dot = '\0'; | |
| free(path_copy); | |
| return scanid; | |
| } |
🧰 Tools
🪛 Cppcheck (2.19.0)
[warning] 238-238: If memory allocation fails, then there is a possible null pointer dereference
(nullPointerOutOfMemory)
🤖 Prompt for AI Agents
In `@src/scan.c` around lines 233 - 243, The helper that extracts a scan id is not
checking strdup return values (path_copy and scanid), causing possible NULL
dereferences in get_scanid_from_path (use of path_copy, basename, scanid,
strrchr). Fix by checking the result of strdup for NULL after creating path_copy
and again after creating scanid; on allocation failure free any allocated
resources and return NULL (or propagate an error) instead of proceeding to
basename/strrchr. Ensure you still free path_copy when appropriate before
returning.
| static void write_progress_to_file(const char *status_path, time_t started, const char *status, int progress) | ||
| { | ||
| if (!status_path || !status) return; | ||
|
|
||
| FILE *fp = fopen(status_path, "w"); | ||
| if (!fp) { | ||
| scanlog("Warning: Could not open status file %s: %s\n", status_path, strerror(errno)); | ||
| return; | ||
| } | ||
|
|
||
| fprintf(fp, "{\"started\":%ld,\"status\":\"%s\",\"progress\":%d}\n", | ||
| started, status, progress); | ||
| fclose(fp); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the main.c file and the clean_old_status_files function
find . -name "main.c" -type fRepository: scanoss/engine
Length of output: 70
🏁 Script executed:
# Search for clean_old_status_files function and how it parses time_t
rg "clean_old_status_files" -A 20 -B 2Repository: scanoss/engine
Length of output: 1503
🏁 Script executed:
# Search for all time_t usages in the codebase
rg "time_t" -B 2 -A 2Repository: scanoss/engine
Length of output: 1500
🌐 Web query:
time_t portability C standard format specifier fprintf
💡 Result:
In ISO C, time_t is only required to be an arithmetic type (it may be an integer or a floating type), and its meaning/encoding is not specified, so there is no fully portable fprintf/printf format specifier for time_t. A strictly conforming program should treat time_t as mostly opaque and only pass it to library functions like localtime/gmtime/difftime. [1], [2]
Portable way (C standard)
Convert to broken-down time and format with strftime:
time_t t = time(NULL);
struct tm tm = *localtime(&t);
char buf[64];
strftime(buf, sizeof buf, "%Y-%m-%d %H:%M:%S %z", &tm);
fprintf(stdout, "%s\n", buf);strftime is designed for formatting time values portably. [3]
If you specifically target POSIX
On POSIX systems, time_t is an integer count of seconds since the Epoch, but it’s still recommended to treat it as opaque unless you control the platform/ABI. [1], [2]
If you do know time_t is an integer type on your targets, a common approach is:
`#include` <inttypes.h>
fprintf(stdout, "%" PRIdMAX "\n", (intmax_t)t);…but this is not ISO-C-portable if time_t is floating or wider than intmax_t. [1]
Sources:
[1] GNU C Library Manual – Time Types (ISO C vs POSIX properties of time_t)
[2] Apple/BSD man page (notes time_t intended to be mostly opaque; use time functions)
[3] CS50 man page – strftime (portable formatting of broken-down time)
🏁 Script executed:
# Get more context on the atol parsing in main.c
rg "atol.*started" -B 5 -A 10Repository: scanoss/engine
Length of output: 687
🏁 Script executed:
# Check if there are any includes of inttypes.h or similar portability headers
rg "inttypes|stdint" -B 2 -A 2Repository: scanoss/engine
Length of output: 4060
Use a portable format for time_t in JSON output and parsing.
%ld is not portable for time_t — on some platforms (e.g., 64-bit time_t on 32-bit systems), it may produce incorrect output. The proposed fix using %lld with a (long long) cast is more practical but still not ISO-C-portable.
For the JSON format containing a numeric timestamp, a more robust approach is to use intmax_t with the PRIdMAX macro from <inttypes.h>:
fprintf(fp, "{\"started\":%" PRIdMAX ",\"status\":\"%s\",\"progress\":%d}\n",
(intmax_t)started, status, progress);Additionally, the parsing in main.c's clean_old_status_files function also needs updating — it currently uses atol() which has the same portability issue:
time_t started = (time_t)strtol(started_ptr + 10, NULL, 10);This ensures consistency between writing and parsing the timestamp.
🤖 Prompt for AI Agents
In `@src/scan.c` around lines 252 - 265, Replace the non-portable time_t
formatting and parsing by using intmax_t/PRIdMAX for output and strtoimax for
input: in write_progress_to_file (function write_progress_to_file) include
<inttypes.h> and change the fprintf to print (intmax_t)started with the PRIdMAX
macro; in main.c's clean_old_status_files replace the atol/strtol parsing with
strtoimax(...) to obtain an intmax_t and then cast to time_t (time_t started =
(time_t)parsed_val), adding minimal error/overflow checking as appropriate so
write and read use the same portable integer format.
1 participant
Summary by CodeRabbit
Release Notes
-R/--reportto enable batch mode output,-C/--cleanto clear old batch status files,--batch-status IDto check ongoing scan progress, and--batch-result IDto retrieve completed scan results