refactor(FileLock): move platform #ifdef out of the class body - #909
Merged
mrjimenez merged 1 commit intoJun 7, 2026
Merged
Conversation
The Win32 and POSIX implementations were interleaved inside a single #ifdef block that opened mid-constructor and closed at the end of the file, spanning the constructor, destructor, SetLock helper, and all member declarations. Pull the class into a single unified declaration (with #ifdef only around the differing member variable), then define the three methods as inline functions below the class. Each method now contains its own narrowly-scoped #ifdef where the platform paths actually differ. Also fixes a typo in a comment: "Desribtor" -> "Descriptor".
got3nks
approved these changes
Jun 7, 2026
got3nks
left a comment
got3nks
left a comment
Contributor
There was a problem hiding this comment.
LGTM. Refactor is correct and API-stable:
- Class body now unified;
#ifdefnarrowed to the differing member (HANDLE hdvsint m_fd) — no more 90-line#ifdefspanning constructor / destructor /SetLock/ private members. - Out-of-line
inlinedefinitions correctly applied (required for ODR —FileLock.his included byamuleAppCommon.cppandED2KLinkParser.cpp). - Member declaration order preserved on both platforms; constructor init lists match (
: m_ok(false)Win32,: m_fd(-1), m_ok(false)POSIX). - Win32 and POSIX constructor / destructor /
SetLockbodies are line-for-line identical to the originals, just relocated. - Two existing callers (
amuleAppCommon.cpp:143,ED2KLinkParser.cpp:266) untouched. - Bonus typo fix:
Desribtor→Descriptor.
No functional change. CI will confirm the build across platforms.
got3nks
added a commit
to got3nks/amule
that referenced
this pull request
Jun 7, 2026
…ule-project#912) Extends existing categories (preferring extensions over new lines): - Performance/Upload: amule-project#898 SlotAllocation default raised. - Networking & Discovery: wire-parser hardening list extended with amule-project#879/amule-project#882/amule-project#890/amule-project#886; new amuleweb security hardening bullet consolidating ngosang's amule-project#869-amule-project#874 triage (all landed in amule-project#875); amulegui list extended with amule-project#857; shared-folder watcher extended with amule-project#858. - Packaging: Windows installer i18n line extended with amule-project#899. - Internals & Refactoring: new docs-polish + code-quality bullets covering amule-project#851/amule-project#855/amule-project#862/amule-project#888/amule-project#900/amule-project#866/amule-project#867/amule-project#895 and amule-project#909/amule-project#910/amule-project#912. - Translations: new pre-release final-wave bullet covering amule-project#847/amule-project#856/ amule-project#891/amule-project#908/amule-project#860/amule-project#904/amule-project#859/amule-project#863/amule-project#861/amule-project#880/amule-project#911/amule-project#901/amule-project#902/amule-project#889/amule-project#868/amule-project#853. - Bug Fixes & Stability: amule-project#850/amule-project#854/amule-project#878/amule-project#906. - CI: ccache wiring (amule-project#892, amule-project#903) + CodeQL binutils-dev (amule-project#907). Contributors footer gains mifritscher and nguyenhoangminhhieu2004-gif (both first-time contributors). PR index extended through amule-project#912.
mrjimenez
pushed a commit
that referenced
this pull request
Jun 8, 2026
Extends existing categories (preferring extensions over new lines): - Performance/Upload: #898 SlotAllocation default raised. - Networking & Discovery: wire-parser hardening list extended with #879/#882/#890/#886; new amuleweb security hardening bullet consolidating ngosang's #869-#874 triage (all landed in #875); amulegui list extended with #857; shared-folder watcher extended with #858. - Packaging: Windows installer i18n line extended with #899. - Internals & Refactoring: new docs-polish + code-quality bullets covering #851/#855/#862/#888/#900/#866/#867/#895 and #909/#910/#912. - Translations: new pre-release final-wave bullet covering #847/#856/ #891/#908/#860/#904/#859/#863/#861/#880/#911/#901/#902/#889/#868/#853. - Bug Fixes & Stability: #850/#854/#878/#906. - CI: ccache wiring (#892, #903) + CodeQL binutils-dev (#907). Contributors footer gains mifritscher and nguyenhoangminhhieu2004-gif (both first-time contributors). PR index extended through #912.
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
2 participants
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.
Summary
src/FileLock.hhad a structural problem first noted by @gonosztopi in #191: a single#ifdef _WIN32block opened mid-constructor (before the initializer list) and closed at line 171, spanning the constructor, destructor,SetLockhelper, and all private member declarations for both platforms. This made the file hard to read and was flagged as "a really really bad thing".Before: one
#ifdefstarting inside the constructor declaration and ending at the bottom of the file, with the Win32 and POSIX class bodies duplicated in full inside#if/#elsebranches.After: a single unified class declaration at the top (with
#ifdefonly around the differing member variable —HANDLE hdvsint m_fd), followed by threeinlinemethod definitions below the class. Each method contains a narrowly-scoped#ifdefonly where the two platforms actually diverge.Also fixes a typo in a comment:
Desribtor→Descriptor.No functional change.