Skip to content

Latest commit

 

History

History
63 lines (39 loc) · 8.81 KB

File metadata and controls

63 lines (39 loc) · 8.81 KB

Coding guidelines

This project's coding guidelines live in CONTRIBUTING.md. Read it before making non-trivial code changes and follow the conventions it describes (style, formatting, commit messages, patch submission, etc.).

Directory hierarchy — put files where they belong

Synchronet's install directory has a defined structure (documented at https://wiki.synchro.net/dir:). Each sub-directory has a specific purpose, and what a file is determines where it goes — getting this wrong (e.g. dropping a data file in exec/) is a real defect, not a stylistic nit. The defaults below are sub-directories of the install dir (/sbbs, ~/sbbs, c:\sbbs) but are SCFG-configurable, so code must locate them via system.*_dir (system.ctrl_dir, system.data_dir, system.exec_dir, system.text_dir, system.mods_dir) — never a hardcoded path.

Dir system.* Holds Don't put here
ctrl/ ctrl_dir Configuration for components that ship with Synchronet — .ini/.cfg/.dat config and curated/hand-edited data files (e.g. text.dat, chat_llm.ini, the curated llm_external_archives.json). The one place for "how this BBS is set up." generated runtime state; scripts
data/ data_dir Generated run-time data created during normal operation — message bases, user data (data/user/), logs, *.json/*.jsonl state, semaphore files, and generated indexes (e.g. the chat BM25 index data/chat/guru.idx). config; scripts
exec/ exec_dir Executable & interpreted code.exe, Baja .bin/.src/.inc, and JavaScript .js. Not normally modified by the sysop. data/config files of any kind
exec/load/ JS libraries load()ed by other modules (sbbsdefs.js, etc.). standalone modules
mods/ mods_dir Sysop-modified-stock or 3rd-party .js/.bin (and, since v3.21, text/menu files). Shadows exec/ and text/: a file here is used instead of the same-named file there, surviving upgrades. brand-new stock features
text/ text_dir User-displayable text & menu files (text/menu/, .asc/.ans/.msg, avatars). code; config

Practical rule that has bitten this codebase: a curated/config data file (JSON included) goes in ctrl/, generated state goes in data/, and exec/ is for code only. When adding a file or a path that reads/writes one, pick the directory by the file's role and address it with the matching system.*_dir.

Segfaults are bugs — always investigate

Any segfault (or other crash: access violation, abort, stack overflow) of a Synchronet executable — sbbs.dll, jsexec, sbbscon, mailsrvr, ftpsrvr, websrvr, services, ntsvcs, smbutil, chksmb, fixsmb, scfg, sbbsctrl, useredit, syncterm, the SMB or xpdev test binaries, etc. — is a real defect and must be root-caused, not worked around or ignored. This applies whether the crash happens during normal use, in a test, or in a one-off probe (jsexec, a quick MsgBase script, a build's own self-test). "Just don't do that" / "retry / skip the offending input" is not an acceptable resolution.

Investigate the root cause: get a stack trace (debugger, crash dump, core file), identify the offending function and reason (null deref, use-after-free, bad cast, uninitialized memory, struct-size mismatch, ABI break, recursion, etc.), and fix it at the source. If a fix is out of scope for the immediate task, file/track it explicitly so it isn't lost.

Don't dismiss SpiderMonkey 1.8.5 (libmozjs/mozjs185-1.0.dll) crashes as unresolvable "old engine" limitations to be waved off until the SM128 migration. An access violation deep in mozjs185 frames — even right after an "out of memory" / "Failed to create new context" log line — is usually a real, fixable bug in our code's use of the engine (a lifetime/NULL/threading mistake), not an engine internal. Case in point: sbbsctrl/sbbscon crashed with an AV in mozjs185!JS_TriggerAllOperationCallbacks, easy to write off as inherent 32-bit OOM fragility — but the actual cause was a missing NULL check in our js_rtpool.cpp (jsrt_GetNew() listed a NULL JS_NewRuntime() result that the trigger thread then dereferenced; GitLab #1152, fixed in fab8b1f1d). SM128 would make true OOM catchable, but it was not what fixed that crash. Root-cause first. On Windows, WER auto-saves a minidump to %LOCALAPPDATA%\CrashDumps\ (C:\Users\<user>\AppData\Local\CrashDumps\); analyze it with cdb (C:\bin\windbg\pkg\{x86,amd64}\cdb.exe — use the x86 build for a 32-bit dump): cdb -z <dump> -c ".ecxr; !analyze -v; kb; q" symbolizes the faulting instruction and our call stack even though mozjs185 itself ships no symbols.

Don't create identifiers in the reserved namespace

The C and C++ standards reserve certain identifier forms for the implementation. User code (including macros, header guards, typedefs, function/variable names) must not use them:

  • Any identifier beginning with an underscore followed by an uppercase letter (e.g. _FOO, _RATELIMIT_HPP).
  • Any identifier containing a double underscore anywhere (e.g. FOO__BAR, __foo).
  • At file scope, any identifier beginning with a single underscore (e.g. _foo).

This includes header include guards. Use the trailing-underscore form instead — e.g. RATELIMIT_HPP_, SBBS_FOO_H_ — or any other form that doesn't start with an underscore or contain __. Much of the existing tree predates this rule and still uses leading-underscore guards; don't mass-rewrite, but never introduce a new one, and flip a header's guard to a conforming form whenever you're already editing it.

Write portable C/C++ — the project builds with MSVC, GCC, and Clang

Synchronet's C/C++ sources have to compile cleanly under three main toolchains (MSVC on Windows, GCC and Clang on Unix-likes), plus to a lesser extent Borland C++ for the legacy sbbsctrl / useredit GUIs. MSVC tends to be the most permissive of the bunch; constructs MSVC accepts (with at most a warning) are often hard errors under GCC and Clang. A successful MSBuild compile on Windows is therefore not a full verification — it only proves the change isn't broken under the most lenient compiler.

The portability gotcha I've actually been bitten by:

  • goto jumping past a local variable's initialization is a hard error under GCC and Clang (error: jump to label 'X' ... crosses initialization of 'Y') whenever Y is still in scope at label X. MSVC accepts it. Fix by either (a) hoisting Y's declaration above any goto that targets a label past it, or (b) wrapping Y in its own brace block so it falls out of scope before the label. Real-world pattern: an early-exit goto cleanup; near the top of a function that skips later T var = init(); declarations whose label is at the bottom — happened in chat.cpp's chat_llm_session() / chat_llm_multinode_turn() (introduced in commit 683147f9c, fixed in 2a0ee8dd1).

Other areas where MSVC is more forgiving than GCC/Clang:

  • Reaching the end of a non-void function without a return — GCC/Clang error, MSVC warning.
  • Designated initializers in C++20 — GCC/Clang require declaration order; MSVC accepts arbitrary order. Use declaration order for portability.
  • Narrowing conversions in braced-init lists (int x{some_long}) — GCC/Clang error, MSVC warning.
  • -Werror is in effect on some build targets, so a warning under one compiler can become an error elsewhere. Don't ignore warnings just because the build you ran passed.

Borland is even more conservative than MSVC on modern C/C++ features and largely affects only the two GUI sub-projects; if you're touching those, see src/sbbs3/CLAUDE.md and the synchronet-build skill's Borland section. For everything else, the practical rule is: write code that compiles cleanly under GCC/Clang first; MSVC will follow.

When you make a .c/.cpp change and can only verify it under MSVC, say so explicitly when offering the commit so the next compile under GCC/Clang isn't a surprise.

Release notes — docs/v3*_new*

Each release has a "what's new" file in docs/ (docs/v321_new.txt, docs/v320_new.txt, …, and the in-progress docs/v322_new.md). When recording changes for an upcoming release, update the file for that version (one greater than the last shipped release).

  • Match the existing v3xx_new files in style, category structure, and depth. Group changes under the same kinds of headings the prior notes use, written for sysops/users — what changed and why it matters — not as a raw commit log.
  • Keep entries brief. Concise, one-line-style bullets; no paragraphs or commit-message dumps.
  • To find what belongs in the notes, review the merged changes on master since the previous release tag (git / GitLab history); data/gitpush.jsonl can help enumerate them.