fix(portability): clean up unsafe uses of long for LLP64 - #65
Merged
Conversation
Following the portability audit in amule-project#41, `long` is 32-bit on LLP64 (Windows 64-bit) but 64-bit on LP64, causing pointer truncation, file-size truncation, and accidental narrowing through APIs typed as `long`. This commit fixes every site flagged in the issue. ## Confirmed bugs - WebServer.cpp / php_core_lib.cpp: ftell's `long` return wraps for >2 GiB template files on Win64. Capture into a 64-bit local with bounds check before narrowing. Won't bite a real user (templates are tiny) but fixes the type confusion at the source. - TextClient.h: `unsigned long lFileSize` widened to `uint64`, with matching `%llu` printf format in TextClient.cpp. Fixes amulecmd's mis-display of any shared file > 4 GiB on Win64. ## InternalEvents API widening - `CMuleInternalEvent::SetExtraLong` / `GetExtraLong` (carrying a `long m_value`) renamed to `SetExtraInt64` / `GetExtraInt64` with underlying type `int64_t`. The existing `HTTPDownload.cpp:312` caller already routes a `wxFileOffset` (potentially > 4 GiB) through it via an explicit `(long)` cast that silently truncated on Win64; the cast is now dropped. - All 16 callsites updated to the new name across `amule.cpp`, `amule-remote-gui.cpp`, `AsyncDNS.cpp`, `HTTPDownload.cpp`. Drop the stale `(long)m_result` cast at HTTPDownload.cpp:417 too. ## Pointer-to-integer casts → uintptr_t - FileArea.cpp: pointer arithmetic in the SIGBUS handler used `unsigned long`, which would truncate pointers on Win64. POSIX-only path today, but worth fixing. - MuleDebug.cpp: same in the bfd backtrace `s_pie_base` subtraction. ## Process IDs → int - `webserver_pid` (amule.h, TerminationProcessAmuleweb.h/.cpp, amule.cpp call sites) changed from `long` to `int`. POSIX pid_t is typically `int`, Windows DWORDs fit in `int`, and `int` avoids `pid_t`'s portability wart on MSVC. Format strings updated from `%ld` to `%d`; the wxExecute return is now narrowed explicitly. - AppImageIntegration.cpp: drop the `static_cast<long>(getpid())` -- getpid returns pid_t (int on every aMule target), no cast needed; format updated to `%d`. ## PartFileConvert intermediate - Switch `long l + ToLong + (unsigned) cast` to `unsigned long l + ToULong + uint32 cast`. File indices are small in practice; the new pattern makes the type intent explicit. Refs amule-project#41. Build verification: macOS `cmake --build` of amule + amuled + amulecmd + amuleweb + amulegui completes warning- and error-free on the patched tree.
The portability commit changed two msgids -- the two "Terminating / Killing amuleweb instance with pid '%d' ... " log messages -- from `%ld` to `%d` to match the new `int` type of `webserver_pid`. CFormat is type-safe so the rendered output is unchanged, but the extracted msgids drift so the .pot and every .po has to be regenerated. msgmerge marked the two updated entries `#, fuzzy` in each catalog that previously translated them. The only difference between the old and new msgstr is the same %ld -> %d swap, so this is a mechanical fix: each affected msgstr has its `%ld` replaced with `%d` and the fuzzy marker dropped. No human translation work needed -- all languages keep their existing wording. Catalogs that already had the entries untranslated, and unrelated pre-existing fuzzy entries (e.g. the English (U.S.) marker from PR amule-project#60), are left untouched.
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.
Summary
Fixes #41.
longis 32-bit on LLP64 (Windows 64-bit) but 64-bit on LP64 — every site flagged in @ngosang's audit is fixed here in one PR. Built clean on macOS (amule + amuled + amulecmd + amuleweb + amulegui, no warnings); CI will verify Ubuntu / mingw / macOS in all configurations.Commit 1 — source fixes (+82/-41)
ftellcapture inWebServer.cppandphp_core_lib.cppnow uses a 64-bit local + bounds check;lFileSizeinTextClient.hwidened touint64with%lluformat inTextClient.cpp.CMuleInternalEventAPI:SetExtraLong/GetExtraLongrenamed toSetExtraInt64/GetExtraInt64, backing typeint64_t. Dropped the silently-truncating(long)expectedcast atHTTPDownload.cpp:312. All 16 callsites updated.unsigned long→uintptr_tinFileArea.cppandMuleDebug.cpp.webserver_pidlong→intacrossamule.h,TerminationProcessAmuleweb.h/.cpp; format strings%ld→%d;wxExecutereturn narrowed explicitly. Droppedstatic_cast<long>(getpid())inAppImageIntegration.cpp.PartFileConvert:ToLong + (unsigned)cast→ToULong + uint32 cast.Commit 2 — translation catalog regen (+780/-780)
Output of
./scripts/update-po.sh. Thewebserver_pidformat change altered two msgids (%ld→%d), which msgmerge marks fuzzy in every catalog that previously translated them. Since the format specifier is the only difference and the format specifier appears identically in every translated msgstr, I un-fuzzied them mechanically:%ld→%din the msgstr, drop thefuzzymarker. No human translation work. Unrelated pre-existing fuzzy entries (e.g. theEnglish (U.S.)marker from #60) are untouched.The bulk of the diff is source-reference renumbering (
#:comments) because the source-line shifts in commit 1 ripple through every catalog's reference comments. Drift check (msgcat --no-wrap, strip headers +#:refs) passes cleanly.Test plan