ListDetector - module automatically reloads #62
Conversation
…n the configuration file
There was a problem hiding this comment.
Pull Request Overview
This PR implements automatic configuration reloading functionality for the ListDetector module. When changes are detected in the configuration file, the module automatically restarts itself to apply the new configuration.
- Adds a new ConfigReloader class that monitors configuration files for changes
- Implements file timestamp-based change detection with periodic checking
- Integrates automatic restart functionality using process replacement via execv()
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| modules/listDetector/src/main.cpp | Integrates ConfigReloader into the main application, copies argv for restart capability |
| modules/listDetector/src/configReloader.hpp | Declares ConfigReloader class with file monitoring and restart functionality |
| modules/listDetector/src/configReloader.cpp | Implements file change detection, monitoring thread, and process restart logic |
| modules/listDetector/src/CMakeLists.txt | Adds configReloader.cpp to the build configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| argv_copy[argc] = nullptr; | ||
| strcpy(argv_copy[i], argv[i]); | ||
| } |
There was a problem hiding this comment.
Line 111 sets argv_copy[argc] to nullptr inside the loop, but this should be done after the loop completes. This will overwrite the null terminator on every iteration except the last.
| argv_copy[argc] = nullptr; | |
| strcpy(argv_copy[i], argv[i]); | |
| } | |
| strcpy(argv_copy[i], argv[i]); | |
| } | |
| argv_copy[argc] = nullptr; |
| char** argv_copy = new char*[argc + 1]; | ||
| for (int i = 0; i < argc; ++i) { | ||
| argv_copy[i] = new char[strlen(argv[i]) + 1]; | ||
| argv_copy[argc] = nullptr; | ||
| strcpy(argv_copy[i], argv[i]); | ||
| } |
There was a problem hiding this comment.
Memory allocated with 'new' is never freed, causing a memory leak. Consider using smart pointers or RAII containers like std::vectorstd::string instead of manual memory management.
| auto previousTimestamp = getLastModified(filePath); | ||
| while (true) { | ||
| sleep(CHECK_INTERVAL); | ||
| auto currentTimestamp = getLastModified(filePath); | ||
| if (currentTimestamp != previousTimestamp && currentTimestamp != -1) { | ||
| previousTimestamp = currentTimestamp; | ||
|
|
||
| this->restart(); | ||
| } | ||
| } |
There was a problem hiding this comment.
The infinite loop will continue running even after restart() is called. Since restart() calls execv() which replaces the current process, the code after restart() should be unreachable, but if execv() fails, the loop will continue indefinitely without any error handling.
| void ConfigReloader::restart() | ||
| { | ||
| execv(argv[0], argv); | ||
| } |
There was a problem hiding this comment.
No error handling for execv() failure. If execv() fails, the function returns normally but the process hasn't been replaced, potentially leading to unexpected behavior. Consider adding error handling or at least logging when execv() fails.
3 participants
ListDetector checks for a change in the configuration file, if a change is detected, the module automatically restarts itself