Check for csv file on set path #4
Conversation
WalkthroughThe changes introduce a file existence check in the CSV configuration logic, ensuring the specified file exists before proceeding. Platform-specific macros are added for compatibility. Separately, error handling for file opening failures is removed from the CSV reader initialization functions, allowing potential Changes
Estimated code review effort2 (~15 minutes) Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
csv_reader.c (1)
78-78: Same file opening issue as in csv_reader_init_with_config.Like the previous function, this removes error handling for
fopen()failure, which could result in a reader with a NULL file pointer being returned.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
csv_config.c(5 hunks)csv_reader.c(8 hunks)
🧬 Code Graph Analysis (2)
csv_reader.c (2)
arena.c (2)
arena_create(16-30)arena_destroy(53-61)csv_parser.c (1)
read_full_record(223-295)
csv_config.c (1)
arena.c (1)
arena_alloc(63-80)
🧰 Additional context used
🧬 Code Graph Analysis (2)
csv_reader.c (2)
arena.c (2)
arena_create(16-30)arena_destroy(53-61)csv_parser.c (1)
read_full_record(223-295)
csv_config.c (1)
arena.c (1)
arena_alloc(63-80)
🔇 Additional comments (5)
csv_reader.c (3)
48-48: Clean whitespace formatting.Good cleanup of trailing whitespace.
Also applies to: 51-51, 57-57, 60-60, 68-68
144-144: Clean whitespace formatting throughout the file.Good cleanup of trailing whitespace and blank lines.
Also applies to: 168-168, 173-173, 182-182, 196-196, 207-207, 212-212, 214-214, 215-215, 224-224, 230-230, 243-243, 246-246, 248-248, 256-256, 265-265, 275-275, 283-283, 288-288, 291-291, 293-293
16-16: Safe handling of NULL file pointers confirmedThe implementation of
read_full_recordand all CSV reader APIs guard against aNULLFILE*, so iffopen()fails the code will not crash:• csv_parser.c –
read_full_recordimmediately returnsNULLiffileisNULL(lines 202–205).
• csv_reader.c – All public functions (csv_reader_next_record,csv_reader_get_record_count,csv_reader_get_position,csv_reader_seek,csv_reader_has_next,csv_reader_rewind,csv_reader_free) checkreader->fileis non-NULL before using it.No further error‐handling changes are required here.
csv_config.c (2)
1-8: Good cross-platform compatibility for file access.The platform-specific includes and macro definitions properly handle the differences between Windows (
_access) and Unix-like systems (access) for file existence checking.
15-15: Clean whitespace formatting.Good cleanup of trailing whitespace throughout the file.
Also applies to: 18-18, 30-30, 42-42, 46-46, 170-170
| if (access(path, F_OK) != 0) { | ||
| printf("Error: File %s does not exist!", path); | ||
| exit(1); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider less aggressive error handling than exit(1).
While the file existence check aligns with the PR objectives, calling exit(1) directly is quite aggressive and prevents graceful error handling by the caller. Additionally, there are a couple of issues:
- The error message lacks a newline character
- The validation occurs before checking if
pathis NULL (though access() should handle NULL safely)
Consider returning an error code instead of terminating the program:
-void csv_config_set_path(CSVConfig *config, const char *path) {
+int csv_config_set_path(CSVConfig *config, const char *path) {
+ if (!path) return 0;
+
if (access(path, F_OK) != 0) {
- printf("Error: File %s does not exist!", path);
- exit(1);
+ printf("Error: File %s does not exist!\n", path);
+ return 0;
}
if (config && path) {
strncpy(config->path, path, MAX_PATH_LENGTH - 1);
config->path[MAX_PATH_LENGTH - 1] = '\0';
}
+ return 1;
}📝 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.
| if (access(path, F_OK) != 0) { | |
| printf("Error: File %s does not exist!", path); | |
| exit(1); | |
| } | |
| int csv_config_set_path(CSVConfig *config, const char *path) { | |
| + if (!path) { | |
| + return 0; | |
| + } | |
| if (access(path, F_OK) != 0) { | |
| - printf("Error: File %s does not exist!", path); | |
| - exit(1); | |
| + printf("Error: File %s does not exist!\n", path); | |
| + return 0; | |
| } | |
| if (config && path) { | |
| strncpy(config->path, path, MAX_PATH_LENGTH - 1); | |
| config->path[MAX_PATH_LENGTH - 1] = '\0'; | |
| } | |
| + return 1; | |
| } |
🤖 Prompt for AI Agents
In csv_config.c around lines 121 to 125, the code checks file existence but
calls exit(1) on failure, which is too aggressive and prevents graceful error
handling. Also, the error message lacks a newline and the NULL check for path is
missing. Fix this by first checking if path is NULL and returning an error code
if so, then check file existence with access(), and if the file does not exist,
print the error message with a newline and return an error code instead of
calling exit(1). This allows the caller to handle errors more gracefully.
1 participant
In this PR I made sure that the csv file exists from
csv_config_set_path()before any further steps.If the file does not exist, we show an error message and exit, we don't need to go any further, this check allows me to remove code that checks the file every time as we are sure now from the beginning that it exists.
Summary by CodeRabbit
New Features
Bug Fixes
Style