fix: do not report errors for configuration file errors. #162
Conversation
WalkthroughThe pull request modifies the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
🧹 Nitpick comments (4)
src/kiwi.cc (4)
94-94: Fix typo in error messageThe word "dose" should be "does" in the error message.
- std::cerr << "Configuration file [" << argv[1] << "] is not accessible or dose not exist.\n"; + std::cerr << "Configuration file [" << argv[1] << "] is not accessible or does not exist.\n";
88-96: Enhance error reporting with specific failure reasonThe current error message doesn't distinguish between inaccessibility and non-existence. Consider using
errnoto provide more specific error information.} else { - std::cerr << "Configuration file [" << argv[1] << "] is not accessible or dose not exist.\n"; + std::cerr << "Configuration file [" << argv[1] << "]: " << strerror(errno) << "\n"; return false; }
88-96: Consider using consistent error reporting styleThe codebase uses the
ERRORmacro elsewhere for error reporting (e.g., line 146). Consider using the same pattern here for consistency.} else { - std::cerr << "Configuration file [" << argv[1] << "] is not accessible or dose not exist.\n"; + ERROR("Configuration file [{}] is not accessible or does not exist", argv[1]); return false; }
89-89: Verify file existence before checking accessibilityThe current implementation uses
access()which might not distinguish between non-existence and permission issues. Consider checking existence first.- if (::access(argv[1], R_OK) == 0) { + struct stat st; + if (stat(argv[1], &st) == 0 && S_ISREG(st.st_mode) && ::access(argv[1], R_OK) == 0) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/kiwi.cc(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build_on_ubuntu
- GitHub Check: build_on_macos
|
macos的ci tcl测试失败了 看一下 |
The ci tcl test of macos failed. Take a look |
|
ai的建议看一下 |
Take a look at AI’s suggestions |
#95
Summary by CodeRabbit