Conversation
Bumped version and copyright dates
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughCanonicalization and relative-resolution for module paths were added: module matching now compares filenames against canonicalized paths; GetPath was rewritten to resolve relative paths against the injector binary directory. EjectLib parameter type changed to std::wstring. A spell-check dictionary entry was added and version/date strings updated. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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.
🧹 Nitpick comments (1)
Injector/Injector.cpp (1)
273-276: Consider handling paths longer thanMAX_PATH.If the loader path exceeds
MAX_PATHcharacters,GetModuleFileNamewill truncate and setERROR_INSUFFICIENT_BUFFER. The current check correctly throws, but the error message could be more specific about truncation. This is a minor edge case on most systems.♻️ Optional: More specific error message
if (!GetModuleFileName(NULL, &LoaderPath[0], static_cast<DWORD>(LoaderPath.size())) || GetLastError() == ERROR_INSUFFICIENT_BUFFER) - throw std::runtime_error("Could not get path to loader."); + throw std::runtime_error("Could not get path to loader (path may exceed MAX_PATH).");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Injector/Injector.cpp` around lines 273 - 276, The error handling around GetModuleFileName using the LoaderPath buffer (std::vector<TCHAR> LoaderPath(MAX_PATH)) should distinguish a truncated path due to ERROR_INSUFFICIENT_BUFFER from other failures and provide a clearer message; update the block that calls GetModuleFileName (and checks GetLastError()) to detect ERROR_INSUFFICIENT_BUFFER specifically and throw a runtime_error that mentions the path was truncated/too long (or retry with a larger buffer/long-path API), referencing the LoaderPath variable and the GetModuleFileName call in your update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Injector/Injector.cpp`:
- Around line 273-276: The error handling around GetModuleFileName using the
LoaderPath buffer (std::vector<TCHAR> LoaderPath(MAX_PATH)) should distinguish a
truncated path due to ERROR_INSUFFICIENT_BUFFER from other failures and provide
a clearer message; update the block that calls GetModuleFileName (and checks
GetLastError()) to detect ERROR_INSUFFICIENT_BUFFER specifically and throw a
runtime_error that mentions the path was truncated/too long (or retry with a
larger buffer/long-path API), referencing the LoaderPath variable and the
GetModuleFileName call in your update.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Injector/Injector.cpp (1)
251-291: Well-structured path resolution with good fallback behavior.The two-phase resolution (CWD first, then injector directory) provides good backward compatibility. The comments clearly document the intent.
Minor consideration: Line 281 assumes
GetModuleFileNamereturns a path containing a backslash. While this is true in practice (it returns absolute paths), a defensive check would make this more robust:🔧 Optional defensive check
std::tstring LoaderDir(&LoaderPath[0]); -LoaderDir = LoaderDir.substr(0, LoaderDir.rfind(_T("\\")) + 1); +auto LastSep = LoaderDir.rfind(_T('\\')); +if (LastSep != std::tstring::npos) + LoaderDir = LoaderDir.substr(0, LastSep + 1); +else + LoaderDir.clear(); LoaderDir.append(ModuleName);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Injector/Injector.cpp` around lines 251 - 291, The code assumes GetModuleFileName(...) yields a path containing a backslash and calls LoaderDir.rfind(_T("\\")) without checking the result; update the logic in Injector::GetPath to defensively find the last path separator (use find_last_of on both '\\' and '/' or verify rfind didn't return npos) before calling substr, and handle the no-separator case (e.g., treat LoaderDir as empty directory or use full LoaderDir appropriately) so LoaderDir.substr(...) cannot throw or produce an invalid range; reference the GetModuleFileName call, the LoaderDir variable, and the LoaderDir.rfind(...) usage when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Injector/Injector.cpp`:
- Around line 251-291: The code assumes GetModuleFileName(...) yields a path
containing a backslash and calls LoaderDir.rfind(_T("\\")) without checking the
result; update the logic in Injector::GetPath to defensively find the last path
separator (use find_last_of on both '\\' and '/' or verify rfind didn't return
npos) before calling substr, and handle the no-separator case (e.g., treat
LoaderDir as empty directory or use full LoaderDir appropriately) so
LoaderDir.substr(...) cannot throw or produce an invalid range; reference the
GetModuleFileName call, the LoaderDir variable, and the LoaderDir.rfind(...)
usage when making the change.
1 participant
Summary by CodeRabbit
Bug Fixes
Chores
Configuration