General polishing of code organization and documentation - #76
Merged
Conversation
- Separate definitions and declarations in web socket events asbtractions;
- Fully separate declarations and definitions in IO;
There was a problem hiding this comment.
Pull request overview
This PR is a broad refactor/polish pass across MSAPI’s synchronization, I/O RAII helpers, and inline/header organization, with accompanying documentation style normalization and test updates. It primarily renames/reshapes abstractions (Pthread → Lock, ExitGuard → Guard, FileDescriptor/Directory guards → FileGuard/DirGuard) and restructures .inl files to better separate declarations and definitions.
Changes:
- Replaced the old
Pthreadsynchronization module with the newLockmodule and updated usages across library code and tests. - Simplified IO RAII wrappers and namespaces (
FileGuard,DirGuard) and adjusted call sites accordingly. - Reorganized inline protocol/server code (notably WebSocket events and recv buffer) and updated documentation blocks to the project’s header-style comment format.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/io/source/io.inl | Adds compile-time static_assert checks and updates IO guard type names (DirGuard, FileGuard). |
| tests/unit/authorization/source/authorization.inl | Updates file descriptor RAII wrapper usage and normalizes comment block style. |
| tests/integration/webSocketProtocol/source/observer.inl | Updates atomic lock guard usage to the new Lock::Atomic::Guard. |
| tests/integration/webSocketProtocol/source/node.inl | Replaces Pthread::AtomicLock with Lock::Atomic and updates guard usage. |
| README.md | Updates module documentation link/label from Pthread to Lock. |
| library/source/server/server.h | Switches include to lock.inl and updates member lock types (Lock::Atomic, Lock::AtomicRW). |
| library/source/server/server.cpp | Updates guard types/usages and section header naming. |
| library/source/server/recvBuffer.inl | Moves constructor definition out-of-line and updates IO FD guard usage (FileGuard). |
| library/source/server/authorization.inl | Switches synchronization primitives to Lock and normalizes documentation blocks. |
| library/source/server/application.cpp | Section header rename to reflect nested type (Application::Parameter). |
| library/source/protocol/webSocketEvents.inl | Major reorganization: separates declarations/definitions, replaces Pthread locks with Lock, introduces concepts and moved inline bodies. |
| library/source/protocol/webSocket.inl | Documentation style normalization and replaces fragmented-data lock type with Lock::Atomic. |
| library/source/protocol/standard.cpp | Section header rename (“Global”). |
| library/source/protocol/object.cpp | Section header rename (“Global”). |
| library/source/protocol/http.h | Documentation style normalization. |
| library/source/protocol/http.cpp | Section header rename (“Global”). |
| library/source/help/time.cpp | Section header renames for nested Timer types. |
| library/source/help/table.cpp | Section header rename for nested type (TableBase::Column). |
| library/source/help/sha256.inl | Documentation style normalization. |
| library/source/help/sha1.inl | Documentation style normalization. |
| library/source/help/pthread.hpp | Removes the old pthread abstraction header. |
| library/source/help/lock.inl | Adds the new lock abstraction module (mutexes, atomic lock, atomic RW lock). |
| library/source/help/json.h | Documentation style normalization. |
| library/source/help/io.inl | Renames header guard, removes extra namespaces, introduces FileGuard/DirGuard, and reorganizes declarations/definitions. |
| library/source/help/html.cpp | Section header placement/labeling adjustments. |
| library/source/help/helper.h | Documentation style normalization. |
| library/source/help/diagnostic.inl | Adds explicit Declarations/Definitions section separation. |
| library/source/help/autoClearPtr.inl | Converts @test Add unit test. markers to @todo Add unit test.. |
| apps/manager/web/js/test/testRunner.js | Documentation style normalization. |
| apps/manager/web/js/test/serverSimulator.js | Documentation style normalization. |
| apps/manager/web/js/help/helper.js | Documentation style normalization. |
| apps/manager/web/js/help/dynamic.js | Documentation style normalization. |
| apps/manager/source/manager.h | Switches include to lock.inl, updates lock types/usages, and normalizes comment blocks. |
| apps/manager/source/manager.cpp | Updates lock guard usage and section header labels. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
library/source/server/server.h:511
- The local RAII helper is still named
PthreadLockGuardeven though it now wrapsLock::AtomicRW. This makes the code harder to read during the Pthread→Lock migration and conflicts with the PR’s stated renaming intent; rename the guard (and its instance variable) to match the new abstraction.
struct PthreadLockGuard {
Lock::AtomicRW& rwLock;
FORCE_INLINE PthreadLockGuard(Lock::AtomicRW& rwLock) noexcept
: rwLock{ rwLock }
{
}
FORCE_INLINE ~PthreadLockGuard() noexcept { rwLock.ReadUnlock(); }
};
pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, nullptr);
pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, nullptr);
std::pair<Server*, int*> serverAndId = *static_cast<std::pair<Server*, int*>*>(data);
int id{ *serverAndId.second };
LOG_DEBUG("Pthread function for " + RecvProcessingTypeToString_v<Type>
+ " connection " + _S(id) + " is called, PID: " + _S(gettid()));
Server* server{ serverAndId.first };
PthreadLockGuard pthreadGuard{ server->m_alivePthreadsRWLock };
server->ConnectionRecvProcessing<Type>(id);
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mood is: Legacy killer
During thinking about better approach to implement TLS was realized some issues with existed code. I made the decision to polish existed code. This pool request contains:
General polishing of code organization and documentation