The flip: FUNCTION/XFUNCTION/FUN::fun/delim_check and the module
interfaces take `const UTF8 * const fargs[]`. Double-const is
load-bearing: C++ qualification conversion needs const at both pointer
levels, so builder-side `UTF8 *[]` arrays convert implicitly — the
evaluator, the JIT marshaller, and every owner site need zero casts,
and slot reassignment inside bodies becomes a compile error for free.
The conversions: the flip landed first so the compiler enumerated every
violation; this commit is that inventory worked to zero — ~250 sites
across funceval, funceval2, functions, funmath, help, mail, session,
powers, levels, predicates, conf, walkdb, stringutil, timeutil/
date_scan (regenerated, one-line diff), exp3, and mux_main, each
classified per docs/campaign-2136-const-fargs.md's four recipes.
New idioms (functions.h): trim_space_sep_n() — non-destructive trim for
(pointer, length) consumers, so trim-then-scan sites need no copy at
all; FargVec — the argv counterpart of FargCopy for CS_ARGV handlers.
countwords() and DecodeListOfIntegers() rewritten non-destructive.
The flip deleted more than it added: #2157's fun_munge list1 copy, the
engine_com help-topic copy, fun_index's in-place NUL write, and five
const_casts (process_sex x4, sha1_helper). const_cast budget: zero
added.
Trap recorded in the brief: an old-signature definition doesn't fail
the build — it becomes a C++ overload, and the new-signature symbol
stays undefined until dlopen(RTLD_NOW). delim_check, the conn_bridge
bridges, the dbt_spike stub, and exp3::Call were all silently shadowed;
muxscript was the only host that noticed, because netmux's own net.cpp
resolved the flat-namespace lookup. After any signature flip, grep the
old spelling.
Verified: make test EXPECT_CONFIG="jit=yes" (35 passed / 0 failed) and
make test-scenario, including the new tests/scenario/sidefx_fargs.py
that live-probes the class-3 wrappers smoke never touches (pemit/
trigger/link/tel/wipe/destroy).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BAN_LEGACY falls 41 → 23. Convert every ratchet site that can reach
mux_sprintf/mux_snprintf:
nls path assembly, AST bench floats, attrcache sizes, mail folders,
named references, softcode %f (fixed-precision table; no %.*f),
exp3 safe_ltoa, ganl port, websocket 101 handshake.
Left frozen: dbt_test.cpp (22) and dbt_x64_div_harness.c (1) — standalone
test binaries that do not link libmux. Neither is player-facing text.
Convert remaining engine `int x = mux_atoi64(...)` (and explicit
static_cast<int> forms) to int64_t so parse width is not discarded on
any platform. Includes functions/funceval/funmath, mail folder numbers,
comsys charge parse, JIT register indices, and HIR lower mid/left/right.
dbref parse keeps full-width parse then rejects values outside int
range. Channel charge still stores int after an in-range check.
The half of #1717 that could not be converted mechanically.
MAIL: %d messages in folder %d [%s] (%d unread, %d cleared).
Four independent counts in one sentence are four independent plural
decisions, and MN_(s, p, n) selects one form from one n -- in Russian each
of those nouns needs its own. No marking fixes that; the sentence has to
become several.
Now three lines, each governed by one count:
en 1 message in folder 0 [unnamed].
1 unread message.
0 cleared messages.
es 1 mensaje en la carpeta 0 [unnamed].
1 mensaje sin leer.
0 mensajes borrados.
ko 폴더 0 [unnamed]에 메시지가 1개 있습니다.
읽지 않은 메시지가 1개 있습니다.
삭제된 메시지가 0개 있습니다.
Spanish inflects; Korean uses its single form with the counter after the
noun and reorders %2$d / %3$s ahead of %1$d, which is why %N$ (#1623) is
load-bearing here rather than decorative.
It also fixes the English: the old line read "1 messages".
Fixed lines rather than suppressing zeros. A player checking their own
mailbox gets the same three lines every time, and "0 cleared" is
information rather than noise; output whose shape varies with content is
harder for staff scripts than output that does not.
All four call sites -- both engine and both module -- converted together so
the implementations do not drift (#1614).
## @stats deliberately left alone
%d objects = %d rooms, %d exits, %d things, %d players. (%d garbage)
Same defect, different audience: a staff diagnostic scanned at a glance,
where six lines of nearly-always-nonzero counts would be worse than the one
line they replace, and the translation gain is negligible. Recorded rather
than converted so the next reader knows it was considered.
## Catalogue fallout
.pot 27 -> 30 plural entries, 977 msgids. All three catalogues msgmerge'd;
xx repaired to 977/977 (each form prefixed from its own msgid, not
msgmerge's copy of the old one); es and ko given real translations.
es.po 279 -> 282/977 (28.9%)
ko.po 310 -> 313/977 (32.0%)
tests/comsys_conformance went red on the baseline, as designed -- the delta
showed the new lines as CONTEXT rather than divergence, so engine and module
changed in step. Re-blessed after reading it.
make test: Smoke 1561 x3, conformance PASSED, tests/nls 7, ko 4, plural 43.
TESTEXIT=0.
Refs #1419, #1614, #1622, #1623, #1717.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found while translating mail (#1419). The tell was a line wrong in English
before translation is even involved:
MAIL: 1 messages in folder 0 [unnamed] (1 unread, 0 cleared).
MN_() exists for this (#1622) and #1655 converted the @mail/stats family;
this set was missed. A translator cannot work around it -- the msgid has no
msgid_plural, so there is no second form for a catalogue to supply, and es
must pick one wording for all counts while ru and pl need three and can
express none of them. The fix is in the C++, not the .po.
## Converted, 11 sites
mail.cpp, mail_mod.cpp mailbox is full; URGENT MAIL urgent messages
walkdb.cpp universe contains; objects @chowned; objects marked
object.cpp objects @chowned to you
wiz.cpp You toaded ... objects @chowned
command.cpp at most N players logged in
The mail module duplicates are converted alongside the engine's so the two
implementations do not drift (#1614).
## One was a different bug than the issue described
walkdb.cpp built its message as M_("%d objects %smarked") with %s = "" or
"un". That is fragment assembly, not a missing plural: a translator handed
that %s cannot use it, because the prefix is English morphology rather than
a word. Split into two whole sentences, each pluralised -- the #1575 /
#1588 rule. Now renders "1 object marked" / "8 objects unmarked" instead of
a %s a catalogue has no way to fill.
## Two deliberately NOT converted
MAIL: %d messages in folder %d [%s] (%d unread, %d cleared).
%d objects = %d rooms, %d exits, %d things, %d players. (%d garbage)
MN_(s, p, n) selects one form from one n. Four independent counts are four
independent plural decisions, and in Russian each noun needs its own form.
These want rewording, which is a design call rather than a mechanical
conversion, so they are left and recorded rather than half-fixed.
## Catalogue fallout
.pot 18 -> 27 plural entries; all three catalogues msgmerge'd.
xx is `complete` policy, so its nine new entries had to be filled and
unfuzzied or the guard fails -- which it duly did, first try. msgmerge had
copied the old singular-only text into BOTH forms, so msgstr[0] said
"messages"; each form is now prefixed from its own msgid.
es and ko received real translations for all nine.
es.po 190 -> 198/976 (20.3%)
ko.po 229 -> 236/976 (24.2%)
## Verified end to end, at n=1 and n>1
en 1 object marked 8 objects marked
es 1 objeto marcado 8 objetos marcados
ko 사물 1개를 표시했습니다 사물 8개를 표시했습니다
xx [xx] 1 object marked [xx] 8 objects marked
Korean renders identically for both, which is correct: it declares
nplurals=1, so the catalogue's own rule is being honoured rather than
English's imposed on it.
The unconverted multi-count line still reads "1 messages", as documented.
make test: Smoke 1561 x3, tests/nls 7, ko 4, plural 43. TESTEXIT=0.
Refs #1419, #1575, #1588, #1614, #1622, #1655, #1702.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Convert engine @mail list/review summary lines from tprintf to
mux_table display-column layout (From width 16, Sub trunc 25), matching
the module helpers. Extract format_mail_list_line_at on both paths.
Re-bless comsys_conformance for From field padding. B4 multi-line
read/detail forms remain out of grid scope.
Phase 4 B1/B2 removed the blob headers and introduced M_("Num").
Regenerate pot/xx/ko so check_nls stays green (drop dead blobs, add
Num). Also delete engine Spaces() — only the old %-s padding path used it.
@mail/stats, @mail/dstats and @mail/fstats were one invented per-player
summary served identically for all three switches, ignoring both the
[<player>] argument and the wizard gate:
engine There are 1 messages in the mail spool.
module MAIL: 1 messages (0 read, 1 unread, 0 cleared). 10 bytes total.
Different text, different scope, different meaning. Under #1614 the module
is the shipped default, so every game's @mail/stats changed meaning on
upgrade. The engine is the oracle: three cumulative detail levels, an
optional target, and "no target from a wizard" meaning the whole spool.
Deferring was not an option -- returning MUX_E_NOTIMPLEMENTED yields "That
@mail command is not available." rather than falling through -- so the
module implements all three. Engine and module output is now byte-identical
across all six command forms, verified against one shared database.
The count-bearing strings were T(), so mail statistics were untranslatable
AND read "There are 1 messages" -- the #1622 shape. MN_()/M_() fixes both.
Raw snprintf is now a build error at new call sites. It counts bytes, and
C11 7.21.6.5 and 7.21.6.1p8 both truncate mid-sequence; measured, snprintf
turns a 4-codepoint moniker into E6 97 A5 E6 9C AC E8 -- a lone lead byte,
invalid UTF-8, sent to a player. mux_vsnprintf advances a whole codepoint
at a time and stops before emitting a partial one. Every one of these sites
interpolates player-settable text into a fixed buffer.
The reason 280 such sites accumulated is that stringutil.h cannot be
included from a module: its inlines want Ragel tables only the engine links,
so the correct primitive was unreachable at compile time. mux_format.h
declares the exported ones, removing the excuse; check_formats.py then
freezes the count per file so existing code stays green and site #281 fails
the build. Both directions of the ratchet are tested.
Known divergence: the engine charges mudconf.searchcost, which no
module-visible interface exposes. payfor() exempts wizards and every
cross-player query is wizard-gated, so this is reachable only by a
non-wizard running stats on themselves. Filed rather than bent into an ABI
change here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mail/fstats added 1 to every message size:
cchars += MessageFetchSize(mp->number) + 1;
MessageFetchSize returns std::string::size() (mail.cpp:337), so there is no
terminator in the count to begin with and the + 1 is a phantom byte. The
same engine then reported two different sizes for the same message depending
on which command was asked, which is why this is a defect rather than a
convention: /list, mailinfo(size) and the mail module all disagree with it.
Five sites, not the three in the report. The two extra are in the
per-player branch reached by `@mail/fstats <player>` (full == 2), which had
the same + 1 and would have kept disagreeing after a three-site fix.
## Verified on live netmux, not muxscript
Deliberately, because #1637 turned out to be a muxscript artifact: the
engine's From: field is blank there only because
CScriptConnectionManager::TrimmedName is a stub. Anything measured about
the built-in mail display through that harness needs a real socket to be
believed.
Fresh DB, port 7860, netmux -s, two messages of 8 and 6 characters:
before MAIL: There are 2 new msgs ... totalling 16 characters.
after MAIL: There are 2 new msgs ... totalling 14 characters.
@mail/list ( 8) and ( 6) = 14
@mail/fstats 14
@mail/fstats Wizard 14 <- the two extra sites
mailinfo(1,size) 8
All four now agree. Before the fix a single 8-character message reported 9.
Refs #1587, #1614, #1631, #1637.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three defects, two of which share one root cause.
## The module freed the body it had just stored
CMailMod::new_mail_message omits the creation reference the engine's
MessageAdd takes -- and the engine states the contract directly above
itself: "This function returns a reference to the message and the
reference count is increased to reflect that."
Without it the arithmetic on a send is 0 -> Inc per recipient -> Dec
once at the end of mail_to_list. A single-recipient message returns to
zero the instant it is sent and the body is cleared. Two recipients
survived undercounted by one and would die when the first of them
cleared their copy.
Symptom: @mail <n> rendered the header and then answered "This mail
message does not exist in the database", and @mail/stats reported
"0 bytes total" -- because stats sums MessageFetchSize over bodies that
had already been cleared. Fixing the reference fixed both.
The reference is taken only when ALLOCATING. The engine splits these
into two functions; this one does both jobs, so the distinction is
captured on entry. Taking it on the load path would leak every body for
the life of the process, since the per-header Inc in the storage
callback already counts every reference there is.
## It was saved from real data loss by a foreign key
The clear also calls sqlite_wt_delete_mail_body, which is REFUSED:
mail_headers.body_number REFERENCES mail_bodies(number), and a header
still pointed at it. Verified by hand:
sqlite> DELETE FROM mail_bodies WHERE number=0;
Error: stepping, FOREIGN KEY constraint failed (19)
The module discarded that result, so nothing said so. The row survived
by accident and a RESTART read the message back correctly -- which is
why this looked like "mail never worked" rather than "mail loses
bodies", and why it had to be caught within a single session.
That delete now checks its result and logs, as the engine's equivalent
already did.
## The engine added a byte to every message
add_mail_message joined the signature unconditionally:
MessageAdd(tprintf(T("%s %s"), message, execstr))
With no A_SIGNATURE -- most players -- execstr is empty and every body
gained a trailing space. A 32-character message read back as 33 and
mailinfo(...,size,...) faithfully reported the inflated length. The
module already guarded this; the module was the correct one. Both now
report 32.
## The regression test took three attempts, and all three failed the
## same way
Recorded because the pattern matters more than the fix:
1. Asserted through mailreview(). That is fun_mailreview in
functions.cpp -- an ENGINE function reading the engine's store,
which was never the broken one. It would not have caught this.
2. Asserted through @mail, the right path -- but the driver's module
config loaded only comsys_mod, so mail ran on the engine in BOTH
configurations and proved nothing.
3. run_as asserted the Comsys implementation line but not the Mail
one, which is why (2) went unnoticed.
The driver now loads mail_mod and bails when EITHER line is wrong. That
guard immediately caught a literal "\n" in the config heredoc on its
first run, before it could produce a meaningless pass.
Verified in both directions: 13 pass with the fixes; with mail_mod
loaded and the fixes reverted, cases 9 and 10 fail with the module
answering "does not exist in the database" -- the defect itself, not a
size mismatch standing in for it.
make test green: 1561/1561 on all three smoke routes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A few sites wrapped constant prose in tprintf() with no format arguments
(comsys max-aliases message, Lua-module-loaded line, three mail admin
notices). Drop the no-op tprintf and mark with M_() so they enter the
catalog like other constant notify prose. Leave real format strings as
T(). Regenerate pot/xx without fuzzy entries.
Next notify slice: @mail and @email player feedback.
mail.cpp: convert constant raw_notify T("…") to M_ (~98 sites). The
static mailmsg[] invalid-spec table uses N_() for extraction and
mail_msg() for runtime gettext so the table does not need a refresh
pass. Leave tprintf formats, DASH_LINE/MAIL_LINE decoration, and logs
as T().
plusemail.cpp: four constant @email denials → M_().
Half-mark pass clean. Regenerate pot/xx without fuzzy entries.
Replace ~900 typographic \xE2\x80\x.. escapes (curly quotes, en dashes)
and a few \xE2\x80\230 octal workarounds with real UTF-8 in message
strings under mux/modules and mux/src. Leave stringutil and convert
charset mapping tables as explicit byte sequences.
Mechanical hygiene under the opt-in M_() design: replace T("#-1…") and
T("#-2…") with S_() so softcode ABI tokens are obvious in source and
cannot enter a player catalog. ~400 call sites across engine, exp3,
mail, and driver. Assembled/library-spliced diagnostics (plan §4.2)
are unchanged where they are not a single literal.
Completes the sweep the issue called for. mux_atol returns long, which
is 32-bit on LLP64, so every caller silently truncated on Windows. Two
of those were real defects (the truthiness family and cf_size, fixed in
the preceding commits); the rest were latent, waiting for a value large
enough to matter.
Rather than audit 290 sites for whether each can reach 2^31 today, use
the 64-bit parser everywhere and remove the class. A dbref cannot
overflow now, but nothing stops a later caller passing that same site a
timestamp or a byte count.
Pure 1:1 substitution: 285 lines changed, and every removed line
contained mux_atol while every added line contains mux_atoi64. No
control flow, no types, no behaviour beyond the wider parse.
This is a NO-OP on LP64 -- long is already 64-bit on Linux and macOS, so
the generated code there is unchanged. It only widens the parse on
Windows. Narrowing destinations are unaffected either way: `int x =
mux_atoi64(s)` truncates exactly as `int x = mux_atol(s)` did, on both
models.
Left alone: mux_atol itself in mathutil, its declaration, and three
comments that name it. Callers that genuinely want 32-bit semantics can
still ask for them; none appear to.
Verified on Windows: full solution builds clean with no new warnings,
smoke is 1418 passed / 16 failed / 0 crashes / 306 of 306 dispatched --
identical to before the sweep, with the same 16 build-configuration
failures (exp3 module not loaded, hmac/digest behind UNIX_DIGEST).
Spot checks after the change: the boolean family returns 1 for multiples
of 2^32, cf_size round-trips 3000000000 and still reads -1 as unlimited,
and arithmetic, string and list functions are unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review findings on #1191:
1. Re-entrancy. sqlite_load_comsys() calls select_channel() for every
channel_users row, and select_channel() now calls
ensure_comsys_softcode_sync() -- which assigns s_seen_rev only AFTER
the load returns, so the nested call saw the same stale revision and
started the load again. Instrumenting the hook showed it really does
re-enter (depth 2 on a @ccreate + addcom + chanusers probe with the
modules active); it terminated incidentally rather than by design, so
a future change to the loader could turn it into unbounded recursion.
Guard both hooks on mudstate.bSQLiteLoading -- depth is now 1 with the
same results.
2. IID bumps. GetRevision (comsys) and GetRevision/SoftcodeSend (mail)
grow the control-interface vtables, so a stale comsys_mod.so /
mail_mod.so must fail mux_CreateInstance discovery instead of being
called through slots it does not implement. Bump IID_IComsysControl
and IID_IMailControl (the #817 pattern, matching the IID_IMailStorage
bump that went with #1203). Verified the modules still load and the
gen-sync still works after the bump.
Verified: @list modules reports comsys_mod/mail_mod loaded (the PR's
mechanism works -- all four required exports are now present); module
-owned channels are visible to softcode chanusers(); full make test
green, smoke 1407/1407, 0 crashes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Root cause: comsys_mod/mail_mod lacked mux_CanUnloadNow, so ModuleLoad
never set bLoaded and CreateInstance always returned CLASSNOTAVAILABLE —
commands and softcode both used the engine path by accident.
With modules actually loaded:
- Bump module revision on every write-through; softcode reloads engine
maps from SQLite when the revision advances (select_channel, mail_fetch,
MailList, etc.).
- CComsysStorage::SyncChannel sets has_comsys so softcode reload works.
- Channel Sync uses ON CONFLICT DO UPDATE so updates do not CASCADE-wipe
channel_users/player_channels.
- mailsend() goes through IMailControl::SoftcodeSend; bodies and
mail_db_top are write-through for softcode mail_* fields.
Comment-only; no behavior change.
- mail.cpp RemoveItem: drop the inaccurate "Mirror RemoveAll()" claim.
RemoveAll erases the map entry then clears the iterators; RemoveItem
must test lst.empty() first, so it clears then erases. The point of
the fix is that the iterators get cleared at all, not the ordering.
- dbt_compile.h bail_alarm: it is counted per aborted DBT run, not
per command -- JIT-in-JIT nesting (an ECALL into another compiled
program) can bump it more than once as each run on the stack unwinds.
The old "per-command" wording overstated it.
- jit_lua.cpp RunCompiled: document that a mid-run wall-clock alarm now
surfaces as a successful "#-1 CPU LIMITED" result, not MUX_E_FAIL.
That is deliberate: MUX_E_FAIL makes the Lua caller re-run the whole
chunk in the Lua VM -- work we must not do once the command is over
its CPU budget.
Smoke 1319/1319; JIT q-register oracle green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RemoveItem erased the last message and the mail_storage map entry while
m_mi/m_miEnd still pointed into the destroyed std::list, so purge/expire/
retract loops could use invalidated iterators. Clear iterators first,
matching RemoveAll.
Also restore n_cargs on cand/cor mid-chain rollback (avoid double-counting
the carg pool), only push emit_sref on successful emit, include <list> in
mail.h, and label malias capacity clearly.
Two bugs in send_mail introduced by the mail STL storage migration
(d1f84fd19 / ae9ee42ff), found in review of that stack.
1. Lost SQLite write-through for newly sent mail.
d1f84fd19 dropped send_mail's sqlite_wt_insert_mail() call (the raw
pointer it took was about to be deleted by the new AppendItem), and no
later commit restored it -- the function became dead code. MessageAdd
still writes the mail *body* through, so a crash before the next full
sqlite_sync_mail left an orphaned body with no header: the recipient
loses the message. Worse, the header's sqlite_id stayed -1, so the
read-flag and delete write-throughs (which guard on sqlite_id < 0)
silently skipped persistence for that mail until a full sync rebuilt
the tables. Restore the insert on the committed list element (&newm)
after the mailbox-full check, matching the pre-migration behavior.
2. Mailbox-full off-by-one.
ae9ee42ff restructured send_mail to emplace_back the new message before
the capacity check, but the check counts by iterating the mailbox --
which now includes the just-added message. That cut the effective limit
from mail_max_per_player to mail_max_per_player-1 and inflated the
"mailbox is full (N messages)" count by one. Compare total > max (reject
at existing == max) and report total-1.
Verified: full rebuild + smoke suite (1264/1264, 0 crashes; the harness
imports into SQLite and exercises the mail backend paths).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
After vector<MAILBODY> + std::string migration:
- Drop mudstate.mail_db_size (vector handles capacity).
- Clean remaining if (mail_list) style and size sets in clear/grow/sanity.
- Consistent use of mail_list.size() / vector semantics.
No behavior change. Smoke still passes.
- No more `new struct mail` + manual pointer handoff in any load or send path.
Loads now emplace_back directly into per-player std::list<mail>.
- All remaining old mudstate.mail_htab walks for mail data migrated to mail_storage.
- Removed last StringClone/MEMFREE for mail fields (time/subject/tolist).
- Send path now constructs and emplaces cleanly (with pop_back on full mailbox).
- MailList, Append/RemoveAll, sqlite_wt, flatfile, SQLite paths updated.
- Completes the high-value STL modernization for mail messages (strings + ownership).
Builds and smoke (1255/1255) pass.
Complete the mail messages modernization (after strings and bodies).
- Per-player mails now use std::unordered_map<dbref, std::list<mail>> (value
semantics, proper RAII ownership via list).
- MailList refactored to std::list iterators (simpler RemoveItem, no more
manual circular next/prev relinking in most paths).
- Loads (flatfile + SQLite) and send now properly move into the list.
- Removed remaining uses of old mudstate.mail_htab for mail data.
- Updated clear_runtime, purge/orphan, broadcast, etc. to new storage.
- All manual StringClone/MEMFREE for mail fields eliminated.
- next/prev removed from struct mail.
Builds and full smoke suite passes (1255/1255). Greatly reduces manual
memory and list management bug surface in the mail subsystem.
Co-Authored-By: Grok
Continue STL modernization of the mail subsystem (following malias and mail bodies work).
- struct mail now owns std::string for time/subject/tolist (eliminates manual StringCloneLen + MEMFREE).
- Per-player storage prototype uses std::unordered_map<dbref, std::list<mail>> (value semantics, RAII).
- Updated MailList, send/load/remove paths, sqlite_wt, flatfile loads, etc.
- Removed many manual new/delete + string lifetime issues.
- Aligns closer to the mail module's approach.
Builds cleanly. Smoke suite passes.
Prototype following the malias STL modernization.
- Replace the manual MAILBODY array (static MAILBODY*, MAIL_FUDGE,
MEMALLOC + memcpy in mail_db_grow, manual init) with
std::vector<MAILBODY>.
- Update struct mail_body to own std::string m_pMessage (drop
m_nMessage, UTF8*, manual StringCloneLen / MEMFREE).
- mail_db_grow() simplified to vector resize + default construction.
Keeps MAIL_DB_LIMIT guard and mudstate.mail_* for compatibility.
- All call sites updated:
- MessageAdd*, new_mail_message, MessageFetch*, refcounting
- SQLite body sync/load paths (use utf8() helper)
- flatfile load, sanity checks, clear_runtime_mail_data, etc.
- Removed all manual memory management for bodies.
This eliminates another source of leaks, OOM edge cases, and
corruption risks from manual array management in the mail
subsystem (same class as the pre-STL malias code).
Compiles cleanly; smoke suite passes (mail bodies exercised in
load/save/send/receive paths).
Co-Authored-By: Grok (prototype)
Follow-up to 112f7af01 (STL malias refactor). With
`std::vector<std::unique_ptr<malias_t>>`, every entry is always live -- inserts
are always `make_unique`, and nothing stores nullptr -- so the defensive
`if (!m) continue;` / `if (m && ...)` / `if (m)` guards carried over from the
raw-pointer design were dead code that obscured the invariant.
Removed all nine across the eight iteration sites (sqlite_wt_sync_all_aliases,
get_malias, malias_write, do_malias_list_all, do_malias_adminlist, fun_malias,
malias_cleanup, sqlite_sync_mail). This also removes a latent
count/record mismatch in malias_write, where the skip path could have written
fewer records than the `putref(fp, malias.size())` header claimed.
No behavior change. Full rebuild + smoke suite (1264/1264, 0 crashes).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the C-style manual array (malias_t**, ma_size, ma_top,
MA_INC, explicit new[]/delete[], manual shifts on delete, optimistic
size updates before allocation) with:
static std::vector<std::unique_ptr<malias_t>> malias;
Change name/desc from raw UTF8* (StringCloneLen + MEMFREE) to
std::string. The malias_t dtor and all manual lifetime code for
these fields are gone.
- Added small utf8() helper to reduce cast noise at UTF8* boundaries.
- Removed dead malias_compare and the now-unused MA_INC define.
- All creation, lookup, iteration, I/O, cleanup, and error paths
updated to vector/unique_ptr/string semantics.
- RAII now handles cleanup on load/create failure paths and on
erase.
This eliminates several families of bugs that required recent
hardening work (state corruption on alloc failure, leaks on
truncated/OOM loads, OOB reads in delete, manual-free mismatches).
The table is now exception-safe and matches the design already
used in mux/modules/mail/.
Smoke suite (1264/1264) passes.
Follow-up to 315cfb078 (Grok review). Three malias allocation sites set the
count/capacity globals *before* the allocation and left them stale when the
allocation failed, corrupting the malias table state:
1. malias_read (DB load): `ma_size = ma_top = i` was set from the
(attacker-controlled) file count before `new malias_t*[i]`. On a huge count
the allocation throws, is caught, malias stays null, and the function
returns with ma_top still huge. Post-load and runtime paths that loop to
ma_top / index malias[] then walk a null pointer (crash) or spin a huge
loop (DoS). Reachable from a crafted mail.db -- same threat model as the
prior commits. This is the state the 315cfb078 message under-described as a
merely "bounded memory-DoS".
2. @malias add, first-alias branch: ma_size was set to MA_INC before the
allocation; on failure it stayed nonzero with malias null, so the next add
skipped the (re)allocation and dereferenced malias[ma_top].
3. @malias add, grow branch: ma_size was incremented by MA_INC before
allocating the new array; on failure malias still pointed at the smaller
old array while ma_size claimed the larger capacity, so subsequent adds
wrote past the real allocation -- a heap overflow.
Fix: on each allocation failure, reset the count/capacity to match the array
that actually exists (0 for the two fresh-alloc paths; restore the pre-grow
size for the grow path). The SQLite load path already sets its counts only
after a successful allocation and needed no change.
Verified by full rebuild + smoke suite (1264/1264, 0 crashes).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two follow-ups to a056f67d0, same malicious/corrupt-database threat model,
found by review of code paths analogous to the ones fixed there. Verified by
full rebuild + smoke suite (1264/1264, 0 crashes).
1. getstring_noalloc legacy path: static buffer overflow (lib/dbutil.cpp)
a056f67d0 fixed the escaped-string (new_strings=true) branch, but the
legacy (new_strings=false) continued-line branch advanced `p += nLine`
across '\r'-terminated lines with no capacity guard against the static
buf[2*LBUF_SIZE+20]. A crafted legacy string with enough continued lines
marches p past the buffer and the next fgets writes out of bounds. This
branch is reachable from v1/v2 flatfile import (db_rw.cpp lock/field reads
pass new_strings=false). Fix: track nBufferLeft and cap each fgets request
to the space remaining, mirroring the escaped branch.
2. make_numlist direct-recipient path: stack buffer overflow (modules/engine/mail.cpp)
a056f67d0 bounded the *alias-expansion copy loop into aRecip[(LBUF_SIZE+1)/2]
but left the direct-recipient append (aRecip[nRecip++] = target) unbounded.
Because the malias_read clamp lets a single alias fill aRecip to capacity,
a crafted mail.db alias followed by additional recipients in the same @mail
still overflows the stack array. Fix: bound the direct-recipient write with
the same cap, dropping recipients past it (matching the alias loop) rather
than overflowing.
Also assessed but left as-is: malias_read's `new malias_t*[getref()]` takes an
unbounded count, but an oversized/overflowing count throws (bad_alloc /
bad_array_new_length), is caught, and the read loop truncates at EOF -- a
bounded memory-DoS with no natural clamp value, not an overwrite.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Audit of the DB load paths (flatfile reader, mail/malias loader, SQLite
attribute bulk-load) under the malicious/corrupt-database threat model --
the same class as #834-843. Five distinct bugs, all in code the June
hardening (#806/#808/#841/#843) did not reach; each is reachable only from
crafted or tampered database content, not from normal gameplay. Verified by
full rebuild + smoke suite (1264/1264, 0 crashes), which exercises flatfile
export/import and SQLite attribute reads end to end.
1. getstring_noalloc: static buffer overflow (lib/dbutil.cpp)
The escaped-string reader mis-accumulated its output-byte count:
`nOutput = pOutput - p` overwrote the count from prior escape emits
instead of adding to it. On the multi-fgets refill path this
under-decrements nBufferLeft, so the `nBufferLeft <= 0` guard never trips
and the next fgets writes past the 2*LBUF_SIZE+20 static buffer.
Legitimate attributes (<= LBUF) never take the refill path, so this only
fires on a crafted quoted string > ~64KB. Fix: accumulate (+=).
2. make_numlist / malias_read: stack buffer overflow (modules/engine/mail.cpp)
malias_read read the recipient count (numrecep) straight from the file
and pushed that many dbrefs into m->list with no cap, making
m->list.size() attacker-controlled. make_numlist then copied all of
m->list into the fixed stack array aRecip[(LBUF_SIZE+1)/2] with no bound
on nRecip -- a crafted mail.db with numrecep > 16384 overflows the stack
(with attacker-chosen dbrefs) the next time any player mails the alias.
Fix: clamp numrecep to (LBUF_SIZE+1)/2 at load (also bounds the reserve()
that could otherwise exhaust memory on an INT_MAX count), and defensively
bound the copy loop in make_numlist.
3. SQLite bulk-load attribute value: heap buffer overflow (modules/engine/sqlitedb.cpp)
GetAllAttributes/GetBuiltinAttributes passed the raw column blob length to
the cache with no clamp, unlike the write path (cache_put) and the
standalone read path (GetAttribute), both of which clamp to LBUF_SIZE. A
value blob written directly into the SQLite file therefore flows unclamped
to atr_get_str_LEN's `memcpy(s, buff, (*pLen)+1)` into a fixed LBUF_SIZE
buffer, overflowing the heap on first read of the attribute
(Name/look/examine/get). Fix: clamp len to LBUF_SIZE in both bulk-load
functions, mirroring the existing clamps.
4. get_list: infinite loop + unbounded log on truncated flatfile (modules/engine/db_rw.cpp)
get_list had no EOF case: at end-of-file getc() returns EOF, falls to
default, and calls getstring_noalloc(), which makes no progress at EOF
(ungetc(EOF) is a no-op, fgets returns NULL). The for(;;) then spins
forever, pegging a core and emitting log lines. Trigger: a flatfile whose
last object's attribute list is truncated before its '<' terminator. Fix:
add a case EOF that aborts the load.
5. getboolexp1: BOOLEXP subtree leak on malformed v2 lock (modules/engine/db_rw.cpp)
Three error paths in the v2 lock parser returned TRUE_BOOLEXP without
freeing the partially built node/subtree (bad connective, missing ')',
EOF mid lock-string). Bounded and single-shot (db_read aborts the whole
load on corruption; nesting capped at 1024), but still a leak on crafted
input. Fix: free the appropriate node/subtree at each site, matching the
partial-construction state.
Also traced and dropped a sixth candidate (SQLite attribute owner read
unvalidated): the attribute owner is never used to index db[] -- only
compared, read for its flags, or re-stored -- so a corrupt owner is harmless.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The mail loaders read message numbers straight from the (admin-supplied, possibly
corrupt) mail database and use them as mail_list[] indices with no bounds check,
in both the flatfile (load_mail_V5/V6) and SQLite (sqlite_load_mail) paths:
MessageReferenceInc(mp->number) does mail_list[number].m_nRefs++, and
new_mail_message -> MessageAddWithNumber(number) does mail_db_grow(number+1) then
mail_list[number] = … . A huge number makes mail_db_grow's allocation fail (it
returns without growing) and MessageAddWithNumber then writes past the end of
mail_list[] — a heap OOB write.
VERIFIED: a crafted SQLite DB (mail_bodies number=2000000000 + mail_db_top meta)
SIGSEGVs the unpatched build on load; the patched build loads it cleanly.
Reachable via a corrupt/migrated/crafted mail DB (cf. flatfile #806, comsys #841).
Fix: a mail_index_valid(n) helper (0 <= n < mail_db_top) guards every mail_list[]
accessor (MessageReferenceInc/Check/Dec, MessageFetch, MessageFetchSize) —
protecting the load path and any runtime fetch; MessageAddWithNumber rejects
negative/absurd i (avoids i+1 overflow) and re-checks validity after the grow;
mail_db_grow refuses an absurd newtop (> MAIL_DB_LIMIT=64M) to avoid int
size-arithmetic overflow. The accessor guards funnel-protect all three loaders.
parse_msglist ranges are mailbox filters (not mail_list indices), already safe.
Smoke 1255/1255.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Found verifying #783: the SQLite loaders are gated on metadata keys --
sqlite_load_comsys() on has_comsys, sqlite_load_mail() on mail_db_top
-- but ONLY the DbConvert -C/-m import path ever set them. Channel
and mail mutations write through to the tables row-by-row, so a
channel created with @ccreate (or mail sent in-game) on a game that
never went through a -C/-m import landed in the tables and was then
ignored forever: the next warm boot found no gate, fell back to the
legacy comsys.db/mail.db flatfiles, and the runtime came up without
them while the orphaned rows lingered.
The write-throughs now maintain the gates: sqlite_wt_channel() sets
has_comsys, and sqlite_wt_mail_body() keeps mail_db_top current (it
sizes the loader's body array). One cheap metadata upsert alongside
mutations that are already rare.
Verified: a channel @ccreate'd and mail sent in one muxscript session
now warm-load in a fresh process -- db_unload -C/-m exports both,
where before it exported nothing ("has no comsys/mail") despite the
rows sitting in the tables.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CMailMod::shutdown() previously called m_pIStorage->Release() and then
set the field to nullptr in two separate statements. In a future
multi-threaded evaluator a concurrent reader could see the old pointer
between the decrement and the null-write and race into a double
Release. Capture the pointer into a local, null the member field
first, then release via the local, so any racing reader sees null
before the refcount drops.
Also relabel the two "HACK" comments in engine/mail.cpp (sender
attribution for object-sent mail, and the non-player @mail subcommand
gate). Neither is a hack — they encode intentional policy. Replacing
the tags with explanatory notes closes the tracker item without a
behavior change.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Eliminate the ISOUTOFMEMORY macro that unconditionally aborted on allocation
failure. Each of the 23 call sites now handles OOM appropriately:
- Fatal sites (buffer pools, db array, anum table): mux_assert or OutOfMemory
- Recoverable sites (queue, mail, commands, guests, vattrs, config, restart,
forward lists): log the failure and return gracefully
Also fix g_dump_child_pid portability: volatile pid_t -> volatile sig_atomic_t
with explicit casts in ganl_adapter.cpp.
Close integer overflow issue as false alarm (getstring_noalloc uses a bounded
static buffer, so nBuffer+1 cannot wrap).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add LBuf class to alloc.h: an RAII wrapper around alloc_lbuf/free_lbuf
that moves LBUF_SIZE buffers from the stack to the heap pool. Convert
all 108 non-static UTF8 xxx[LBUF_SIZE] stack arrays across 25 source
files. Static BSS buffers (24) are unchanged.
This eliminates LBUF_SIZE from recursive stack frames, making it safe
to increase LBUF_SIZE without risking stack overflow in the evaluation
pipeline.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The malias_t struct used a fixed dbref[100] array, capping mail alias
membership at 99. Replace with std::vector<dbref> and remove the
MAX_MALIAS_MEMBERSHIP constant and numrecep field entirely. Both the
mail module and the server-side fallback code are updated.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rewrite notify_check() from mux_string to raw PUA-encoded UTF-8 lbufs.
The engine now passes PUA strings through; the network layer converts
PUA to ANSI/XTERM/HTML per client. html_escape() replaces encode_Html(),
co_strip_color() replaces export_TextPlain() for @listen matching.
Remove mux_string overloads: raw_notify, raw_notify_html,
send_text_to_player (session.cpp, conn_bridge.cpp, net.cpp).
Convert handle_ears, look_for_exits (predicates.cpp), notify_comsys
(comsys.cpp), and small uses in mail.cpp/match.cpp.
Add extern "C" guards to color_ops.h for C++ inclusion.
593/593 smoke tests pass.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mailsend(recipients, subject, message) sends mail as the executor's
owner with full permission checks, throttle enforcement, and automatic
signature. 6 smoke tests (487 total).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
