================================================================================
COMPILER_FIXES.TXT
Chocolate Doom 2.3.0 - OpenVMS 8.4 Alpha (AXP) port
Astr0baby 2026
================================================================================

Four images are built: DOOM.EXE, HERETIC.EXE, HEXEN.EXE, STRIFE.EXE.  See
DESCRIP.MMS for the build itself; this file is only about the source changes.

Every change is marked in the source with a comment beginning

    // OpenVMS port:

so `SEARCH [.SRC...]*.C "OpenVMS port"` finds all of them.  There are 119 such
comment blocks across 50 files.  Nothing was changed silently.

--------------------------------------------------------------------------------
CONTENTS
--------------------------------------------------------------------------------
  1.  Summary, and the two changes that carry the most risk
  2.  PACKEDATTR and struct packing         (DEC C has no packed attribute)
  3.  FEATURE_MULTIPLAYER off: d_loop.c     (the off path had never been built)
  4.  FEATURE_MULTIPLAYER off: doom/d_main.c and doom/d_net.c
  5.  FEATURE_SOUND off: i_sound.c and doom/m_menu.c
  6.  %GEM-F-ASSERTION: 93 decrement rewrites, and the 33 increments left alone
  7.  SDL_RESIZABLE removed; the 640x480 window
  8.  SDL teardown guarded with SDL_WasInit  (a fatal X error on DECwindows)
  9.  Unchecked allocations
 10.  hexen/mn_menu.c: duplicate definition of demoextend
 11.  m_config.c: where the .CFG files and savegames live
 12.  What was NOT changed, and why

--------------------------------------------------------------------------------
1.  SUMMARY
--------------------------------------------------------------------------------
Change counts, by kind:

    93   decrement-in-controlling-expression rewrites (section 6)
          - DEC C 8.4 AXP internal compiler error; the reason this file exists
    13   feature-off guards for FEATURE_MULTIPLAYER / FEATURE_SOUND (3, 4, 5)
     7   video/window changes, incl. the 640x480 pin (7)
     3   SDL teardown guards (8)
     2   allocation checks (9)
     1   duplicate-definition fix (10)
     1   config-directory branch (11)

Of these, exactly TWO classes change generated code in a way that could alter
behaviour if I got the reasoning wrong, and they are the two documented most
carefully below:

  * The 93 decrement rewrites (section 6).  Each moves a side effect relative
    to a test.  Five distinct transforms are needed, and three of the five are
    WRONG if applied to the wrong shape - notably the idiom the sibling Hexen
    port uses, which silently changes behaviour on unsigned counters.  Section 6
    gives the equivalence argument per class and the harness that checks them.

  * The FEATURE_MULTIPLAYER=off stand-in variables (section 3).  Upstream's
    "off" path does not compile, and making it compile means defining two
    variables that no longer have an owning translation unit.

Everything else is either a compile-or-link necessity with one possible correct
answer, or a deliberate platform policy choice (7, 11) that is called out as
such.

--------------------------------------------------------------------------------
2.  PACKEDATTR AND STRUCT PACKING
--------------------------------------------------------------------------------
NO SOURCE CHANGE WAS NEEDED.  This section exists to record why, because
"the packed attribute is being ignored" is exactly the kind of thing that looks
like a latent file-format bug and would otherwise be re-investigated by the next
person.

[.SRC]DOOMTYPE.H defines

    #ifdef __GNUC__
    #define PACKEDATTR __attribute__((packed))
    #else
    #define PACKEDATTR
    #endif

DEC C is not GCC, so PACKEDATTR expands to NOTHING and every struct marked with
it is laid out at natural alignment.  DESCRIP.MMS compiles with /NOMEMBER
(do not pack), which is consistent with that - it does not try to emulate the
attribute.

The structs so marked are the ON-DISK ones - WAD directory entries, patch and
texture headers, the PCX header, the demo header - so a layout difference would
mean unreadable WADs, not merely wasted bytes.  Every field offset was therefore
compared, packed versus natural, across all four build sets.  Result:

    every member offset is IDENTICAL under both layouts.

That is not luck.  Doom's on-disk structs are built from char/byte arrays,
16-bit shorts and 32-bit ints laid out in an order that is already naturally
aligned; the attribute was defensive, not load-bearing.

The ONLY difference anywhere is TRAILING padding in one struct:

    pcx_t   packed = 129 bytes,  natural = 130 bytes

and that one is harmless because [.SRC]V_VIDEO.C's V_WritePCXfile does not use
sizeof(pcx_t) for the write length.  It computes

    length = pack - (byte *) pcx;

i.e. the distance actually written, so the file it produces is byte-identical
either way.  (Had it used sizeof, every screenshot would have carried one
garbage byte in the middle.)

--------------------------------------------------------------------------------
3.  FEATURE_MULTIPLAYER OFF - [.SRC]D_LOOP.C
--------------------------------------------------------------------------------
Networking is switched off in [.SRC]DOOMFEATURES.H:

    /* #define FEATURE_MULTIPLAYER 1 */

That macro is upstream's own mechanism, and [.SRC]MAKEFILE.AM has a matching
FEATURE_MULTIPLAYER_SOURCE_FILES list, so the intent is clearly that it can be
turned off.  IT HAD NEVER BEEN BUILT THAT WAY.  With it off, upstream does not
compile, and then does not link.  Five changes in this file, plus section 4.

3.1  net_client.h is included UNCONDITIONALLY (d_loop.c:35)

     Upstream includes it inside the FEATURE_MULTIPLAYER block along with
     net_gui.h, net_io.h, net_query.h, net_server.h, net_sdl.h and net_loop.h.
     It is kept out here because it is declaration-only - it pulls in no SDL_net
     - and it declares net_client_connected and drone, which the stand-ins in
     3.2 must agree with.  Including it keeps one declaration in charge of both
     the real and the stubbed build.  The other six stay guarded.

3.2  Stand-in definitions for net_client_connected and drone (d_loop.c:53)

     THE ONE CHANGE IN THIS PORT MOST WORTH RE-READING BEFORE TRUSTING.

     With multiplayer off, no net_*.c is compiled, so nothing defines these two
     variables - yet they are READ from about a dozen places that upstream left
     outside its own #ifdefs, both in d_loop.c and in [.SRC.DOOM]D_MAIN.C:415.
     The options were:

       (a) guard every read site.  That means scattering feature tests through
           the main loop and into game code, in a release where the game code is
           meant to be untouched.
       (b) define the variables once, in the file that owns the loop.

     (b) is what upstream itself does for the analogous "singleplayer" case
     elsewhere, and it is what is done here.  They are deliberately NOT static,
     because net_client.h declares them extern and other objects read them.

     The values are the single-player truth, and they are exactly the values the
     real net code holds in a single-player game:

         boolean net_client_connected = false;   /* no server */
         boolean drone = false;                  /* we are a player, not a
                                                    spectating drone */

     Because they are compile-time-constant false in this build, the
     client/server branches they guard are dead code that DEC C's optimiser
     removes; nothing calls into the absent net layer.

3.3  Guarded function bodies (d_loop.c:346, 440)

     Two function bodies are pure client/server code (they call NET_*), so with
     no net layer they cannot compile at all.  Both are guarded, and both retain
     a single-player return path so every caller still links and still gets a
     correct answer.

3.4  Unused-variable cleanup (d_loop.c:503)

     addr and i are touched only by the multiplayer block, so with it off DEC C
     reports them unused.  Moved inside the guard rather than silenced with a
     cast-to-void, so the real build is unchanged.

--------------------------------------------------------------------------------
4.  FEATURE_MULTIPLAYER OFF - [.SRC.DOOM]D_MAIN.C AND D_NET.C
--------------------------------------------------------------------------------
The same class of problem inside the Doom directory: reads of net state that
upstream left unguarded, and calls to net entry points that no longer exist.
Guarded the same way, keeping the single-player path.

Heretic, Hexen and Strife needed NO equivalent changes: their d_main.c /
h2_main.c reach the net layer only through d_loop.c's already-guarded API.  Doom
is the odd one out because it consults net state directly while parsing
command-line switches.

--------------------------------------------------------------------------------
5.  FEATURE_SOUND OFF - [.SRC]I_SOUND.C AND [.SRC.DOOM]M_MENU.C
--------------------------------------------------------------------------------
Sound is switched off in [.SRC]DOOMFEATURES.H:

    /* #define FEATURE_SOUND 1 */

[.SRC]I_SOUND.C IS STILL COMPILED, and that is the whole trick.  It is the
backend-dispatch layer, not a backend: with FEATURE_SOUND off, every I_*
sound entry point in it degrades to a no-op stub.  The four game directories
make several hundred S_* / I_Sound* calls, and this is what keeps every one of
them compiling and linking without a single #ifdef in the game code.

5.1  Header ordering (i_sound.c:25)

     doomfeatures.h must be seen BEFORE the SDL_mixer include, or the include is
     reached before the macro that is meant to suppress it.  The two headers were
     reordered.

5.2  SDL_mixer include and backend table guarded (i_sound.c:73)

     The sound_modules[] / music_modules[] tables name backends that live in
     files this build does not compile (i_sdlsound.c, i_sdlmusic.c, i_oplmusic.c,
     i_pcsound.c).  Referencing them would be an undefined symbol at link time,
     so the table entries are guarded and the tables come out empty - which the
     surrounding code already handles, since it must cope with "no usable
     backend" anyway.

5.3  I_InitTimidityConfig guarded (i_sound.c:258)

     Defined in i_sdlmusic.c, which is not compiled.

5.4  Shutdown path guarded (i_sound.c:497)

     Upstream guards only part of this; the rest calls into the absent backends.

5.5  I_OPL_DevMessages guarded ([.SRC.DOOM]M_MENU.C:1836)

     Doom's menu has a developer-only OPL diagnostics screen that calls into
     i_oplmusic.c.  Guarded; the menu entry simply does nothing.

--------------------------------------------------------------------------------
6.  %GEM-F-ASSERTION: 93 DECREMENT REWRITES
--------------------------------------------------------------------------------
6.1  THE BUG

DEC C on OpenVMS 8.4 Alpha aborts with an INTERNAL COMPILER ERROR

    %GEM-F-ASSERTION, Compiler internal error - please submit problem report

on a DECREMENT used inside a controlling expression - the test of an if, while,
or do/while.  This is a compiler defect, not a language issue: the code is
valid, portable C89 and builds everywhere else.

Both sibling OpenVMS ports in /opt/CLAUDE/OPENVMS hit it and document it:

    [-.HEXEN]COMPILER_FIXES.txt          "if (var-- <= 0)"
    [-.CHOCOLATE_DUKE]COMPILER_FIXES.TXT "if (pixelsAllowed-- > 0)", 18 sites,
                                         with GEM_LU_MAIN in the traceback

GEM_LU is the loop unroller, which is why the escape hatch in DESCRIP.MMS is
/OPTIMIZE=LEVEL=1 (it disables unrolling) rather than /NOOPTIMIZE.

6.2  SCOPE

93 sites, in 42 files:

    doom      16          heretic   16
    hexen     36          strife    25

By the transform each needed:

    81   hoisted above the "if"        (6.4, class A/B)
     7   kept inside a loop            (6.5, class D)
     5   kept nested under a guard     (6.6, class E)

Found by preprocessing every file in all four build sets and scanning the
token stream, so macro-hidden occurrences could not escape.  Re-run after the
edits: zero remaining.

6.3  THE FIVE IDIOMS AND THEIR TRANSFORMS

Chocolate Doom spells this five ways.  Each needs a DIFFERENT rewrite, and three
of the five are wrong if you apply another one's transform:

    A   --var <op> K      ->  --var;  if (var <op> K)
        !--var            ->  --var;  if (var == 0)
    B   var-- <op> K      ->  old = var;  var--;  if (old <op> K)
        !var--            ->  old = var;  var--;  if (old == 0)
    D   while (var-- != 0) { B }   ->  while (var != 0) { --var; B }
        do { B } while (--var != 0) -> do { B; --var; } while (var != 0)
    E   (short-circuited or nested) -> decrement stays exactly where it was

6.4  CLASS A AND B - HOISTED ABOVE THE "IF"  (81 sites)

Class A is prefix: the test already sees the DECREMENTED value, so evaluating
the decrement one statement earlier and then testing the variable is the same
computation in the same order.

Class B is postfix: the test sees the OLD value while the decrement still
always happens.  The old value must therefore be CAPTURED - reordering alone
would change the comparison:

    // OpenVMS port: "actor->health-- == 0" split; see COMPILER_FIXES.TXT.
    // Postfix, so the test sees the OLD value while the decrement always
    // happens - capture it rather than reordering.
    int oldhealth = actor->health;

    actor->health--;

    if (oldhealth == 0)

    *** DO NOT USE THE SIBLING HEXEN PORT'S IDIOM HERE. ***

That port rewrites postfix tests as

    if (var == 0) ... else var--;

which is right for a SIGNED counter and SILENTLY WRONG for an unsigned one,
because it discards the wrap.  This tree has unsigned cases: Hexen's
mobj_t.args[] is `byte`.  At args[0] == 0, upstream's "!actor->args[0]--" takes
the branch AND leaves 255 behind; the else-form leaves 0.  A harness comparing
all 256 byte values found them differing at exactly one value, v == 0 - so a
bug that shows up only when a counter hits zero, which is precisely when these
counters matter.  Capturing the old value preserves both the branch and the
stored value at every input.

The signed/unsigned split across class B:

    byte (UNSIGNED, wraps 0 -> 255)   hexen args[] - a_action.c, p_enemy.c
    intptr_t (signed)                 hexen special1.i / special2.i, via the
                                      specialval_t union in h2def.h:175
    int (signed)                      everything else

6.5  CLASS D - THE DECREMENT MUST STAY INSIDE THE LOOP  (7 sites)

    [.SRC.HERETIC]AM_MAP.C:1125, 1148, 1164   DrawWuLine
    [.SRC.HEXEN]AM_MAP.C:1023, 1046, 1062     DrawWuLine
    [.SRC.STRIFE]M_SAVES.C:400                trailing-slash stripper

A LOOP CONDITION IS RE-EVALUATED EVERY ITERATION.  Hoisting the decrement above
the loop would run it ONCE and turn a counted loop into an endless one.  So for
the while form the test moves first and the decrement becomes the first
statement of the body:

    // The decrement must stay INSIDE the loop - a loop condition is
    // re-evaluated every iteration, so hoisting it out would decrement only
    // once.
    while (DeltaX != 0)
    {
        --DeltaX;
        X0 += XDir;
        PUTDOT(X0, Y0, &BaseColor[0], NULL);
    }

That runs the body exactly DeltaX times, as before.  The original's failing test
also performed one final decrement, leaving DeltaX at -1; that is unobservable
here because the body never reads DeltaX and the function returns immediately
afterwards.  (Both facts were checked, not assumed.)

For do/while the decrement precedes the test, so it becomes the LAST statement
of the body with the condition testing the variable.

M_SAVES.C:400 is the awkward one - the decrement is the RIGHT operand of && in a
loop condition, so it can neither be hoisted out (re-evaluated each iteration)
nor made unconditional (it must not run when p == str).  Stepping the pointer
back first and testing p[0] is exactly equivalent:

    while (p > str)
    {
        --p;
        if (*p != DIR_SEPARATOR)
            break;
        *p = 0;
    }

6.6  CLASS E - THE DECREMENT MUST STAY NESTED  (5 sites)

    [.SRC.DOOM]HU_STUFF.C:460      !--message_counter, inside if (message_counter)
    [.SRC.DOOM]P_USER.C:338        !--powers[pw_invisibility], likewise
    [.SRC.STRIFE]HU_STUFF.C:407    !--message_counter, likewise
    [.SRC.HEXEN]P_ENEMY.C:4478     parent->args[0]-- <= 0, inside an outer test
    [.SRC.HEXEN]P_ENEMY.C:4606     A_BounceCheck - two nested tests, where
                                   args[3] is decremented ONLY when the args[4]
                                   test succeeds

Here the decrement was already CONDITIONAL - short-circuited by && / ||, or
sitting inside an enclosing if.  Hoisting it above the outer test would make it
run more often and change behaviour.  Each of these keeps its decrement exactly
where it was, and says so:

    // The decrement stays inside the outer test, which is where it always was -
    // it only ever ran when the power was non-zero.
    if (player->powers[pw_invisibility])
    {
        --player->powers[pw_invisibility];

        if (player->powers[pw_invisibility] == 0)
            player->mo->flags &= ~MF_SHADOW;
    }

THE DISTINCTION THAT MATTERS: if the decrement is the LEFT operand of || or &&,
it was ALWAYS evaluated, and hoisting IS safe.  [.SRC.STRIFE]WI_STUFF.C:788
("--cnt || acceleratestage") is that case and is hoisted; the comment there says
why.  Only the RIGHT operand, or a decrement under an outer test, must stay put.

6.7  EQUIVALENCE TESTING

Each class was checked by a harness rather than by inspection alone:

    class A   prefix, over signed -300..300, all six relational operators
    class B   postfix, over signed int AND unsigned byte 0..255
    class C   the sibling Hexen "if (x == 0) else x--" idiom, to demonstrate the
              divergence: differs from upstream at exactly 1 of 256 byte values,
              at v == 0, as predicted in 6.4
    class D   loop iteration counts for DeltaX/DeltaY over 0..299, both loop
              forms
    class E   the trailing-slash stripper, over 13 inputs incl. "", "/", "//",
              "a//" and a string that is nothing but separators

All classes agreed with upstream except C, which is the one this port does not
use.

The new temporaries (oldhealth, oldcount, oldtimer, oldbounce, oldlife) are all
declared at the top of a block, so they are C89-legal; verified by recompiling
all four build sets with -std=gnu89 -Wdeclaration-after-statement, which
reported nothing.

6.8  THE 33 INCREMENT SITES - DELIBERATELY NOT CHANGED

The same tree contains 33 INCREMENT-in-comparison sites, e.g.

    [.SRC]W_MAIN.C            while (++p != myargc && myargv[p][0] != '-')
    [.SRC.DOOM]P_MAP.C        if (++hitcount == 3)
    [.SRC.DOOM]R_DRAW.C       if (++fuzzpos == 50)          (x2)
    [.SRC.DOOM]S_SOUND.C      if (sfx->usefulness++ < 0)
    [.SRC.DOOM]P_MOBJ.C       if (cycle_counter++ > 1000000)
    [.SRC.DOOM]R_BSP.C        while (next++ != newend)      (all four games)
    [.SRC.HEXEN]P_ENEMY.C     if (count++ > 64)
    [.SRC.HEXEN]SC_MAN.C      while (*ScriptPtr++ != '\n')  (x2)
    [.SRC.STRIFE]P_INTER.C    if (players[i].sigiltype++ > 4)
    [.SRC.STRIFE]ST_STUFF.C   if (++st_keypage > 2)
    ... 33 in total, in 27 files

THESE ARE LEFT EXACTLY AS UPSTREAM WROTE THEM.  The reasoning, so the next
person does not have to guess:

  * Both sibling ports document the crash for DECREMENTS only.  The Duke tree
    contains ZERO increment sites, so its clean build is no evidence either way.
    The sibling Hexen tree does contain 8 increment sites - but that port's own
    ALL_FIXES_COMPLETE.txt leaves "[ ] Build completes without errors"
    UNCHECKED, so it is not evidence that they compile.
  * Duke's documented crash was on int32_t pixelsAllowed-- > 0, i.e. a SIGNED
    operand, so the defect is not specific to unsigned arithmetic and one cannot
    argue increments are safe on that basis either.
  * Rewriting 33 correct expressions to dodge a bug not yet observed in this
    direction is itself a risk. Three of them (SC_MAN.C, M_SAVES.C, R_BSP.C)
    increment a POINTER inside a loop condition, which is the easiest kind to
    get subtly wrong, and R_DRAW.C is in the renderer's inner loop.
  * If one does crash the compiler, the cheap fix is per-file flags, not source
    surgery: DESCRIP.MMS defines $(CFLAGS_OPT1) and $(CFLAGS_NOOPT) for exactly
    this, and the sibling notes give the same advice ("Only fix if build
    actually crashes on them").

If it comes to that, work through the list above; the transforms are the mirror
image of 6.3, and the same three traps apply (capture the old value for postfix,
keep loop conditions inside their loop, do not hoist a short-circuited operand).

6.9  IF A NEW %GEM-F-ASSERTION APPEARS

Read the failing PASS NAME in the traceback and change flags before touching the
source:

    GEM_LU_* / PEEL_LOOP   loop unroller; runs only above LEVEL=1
                                                    -> $(CFLAGS_OPT1)
    GEM_DF_* / CSE         LEVEL=3 dataflow          -> $(CFLAGS_OPT1)
    GEM_CX_*               code EXPANDER; runs at EVERY level, so /NOOPTIMIZE
                           can CAUSE it              -> try OPT1, not NOOPT
    GEM_TN_*               register allocation       -> $(CFLAGS_OPT1)
    ME_DEBUGGEN            debug-symbol emission     -> add /NODEBUG/NOTRACEBACK

An unjustified /NOOPTIMIZE cost the sibling Kyra port several build rounds.
Remember to repeat the file's own /INCLUDE=$(INCS_x) when swapping the macro -
the escape-hatch macros carry $(INCS_COMMON), which is wrong for a game file.

--------------------------------------------------------------------------------
7.  SDL_RESIZABLE REMOVED; THE 640x480 WINDOW
--------------------------------------------------------------------------------
A 640x480 window was the requirement for this port.  It needed NO new scaler
code - unlike the sibling Hexen port, which had to grow one - because Chocolate
Doom already ships a software scaler table and one of its stock entries is
exactly right:

    mode_stretch_2x = SCREENWIDTH*2 x SCREENHEIGHT_4_3*2 = 640 x 480

It is an ASPECT-CORRECTING mode (I_Stretch2x + I_InitStretchTables), which is
what turns the 320x200 framebuffer into a 4:3 640x480 window without squashing.
This works for all four games automatically: SCREENWIDTH (320) and
SCREENHEIGHT_4_3 (240) live in the SHARED header [.SRC]I_VIDEO.H, no game
directory overrides them, and I_VIDEO.C is linked into every image.

7.1  Defaults changed (i_video.c:180, 224)

     fullscreen defaults to 0 and screen_width/height to 640/480, rather than
     upstream's fullscreen 320x200.

7.2  The settings are PINNED at the top of I_InitGraphics (i_video.c:1992)

     Defaults alone are not enough, and this is the subtle part: these three are
     config-file-bound variables, and M_BindIntVariable() has already run by the
     time I_InitGraphics is reached.  A DEFAULT.CFG carried over from another
     machine - or from a stock Chocolate Doom install, where fullscreen defaults
     to 1 - would silently win.  So they are forced here:

         fullscreen           = 0     windowed
         aspect_ratio_correct = 1     MUST stay 1 - see below
         screen_width         = 640
         screen_height        = 480

     aspect_ratio_correct is not cosmetic: it selects screen_modes_corrected[],
     which is the ONLY table containing a 640x480 entry.  With it 0,
     I_FindScreenMode(640,480) returns mode_scale_2x = 640x400 instead, i.e. a
     letterboxed window.  Verified at runtime, against the real tables:

         corrected    I_FindScreenMode(640,480) -> 640x480
         uncorrected  I_FindScreenMode(640,480) -> 640x400

     Pinning here rather than only changing the defaults makes the window
     geometry a property of the PORT rather than of the user's config file.

     I_AutoAdjustSettings() runs AFTER the pin, so it was checked that it cannot
     undo it: it calls AutoAdjustWindowed(), which rewrites screen_width/height
     only when the best mode DIFFERS from the request.  Since 640x480 is an
     exact mode, the request is a fixpoint and the call is a no-op.  Confirmed
     at runtime as well as by reading.

7.3  SDL_RESIZABLE is NOT set, unconditionally (i_video.c:1833)

     This is a fixed-size window by design, so advertising a resize grip the
     game will not honour is worse than not offering one.

     Resizing never actually worked in this release anyway.  The event loop's
     resize case (I_GetEvent) tests for "SDL_RESIZABLE", which is a video-mode
     FLAG (0x10) - not an event type.  The event SDL 1.2 actually delivers is
     SDL_VIDEORESIZE, from an unrelated enum.  need_resize is therefore never
     set and ApplyWindowResize() was already unreachable upstream.  Dropping the
     flag only makes the window's behaviour match its appearance.

     Left unconditional rather than behind an #ifdef VMS so that the Linux
     pre-validation build exercises the same code path the VMS build gets.

7.4  Hexen's second video path

     [.SRC]I_VIDEOHR.C is a separate 640x480 4-bit-plane "high resolution" mode
     used only for Hexen's startup screen (ST_START.C) and its network wait
     screen.  It asks SDL for 640x480 directly and is unaffected by the above.
     It is in Hexen's object list and no other game's.

7.5  The window title

     [.SRC]I_VIDEO.C:1131 builds it as

         M_StringJoin(window_title, " - ", PACKAGE_STRING, NULL)

     where window_title comes from each game's own I_SetWindowTitle
     (gamedescription).  PACKAGE_STRING is shared by all four images, so the
     four windows are distinguished by the game's own description, not by the
     package string.  Nothing was changed here; it is noted because it looks
     like a per-game string and is not.

--------------------------------------------------------------------------------
8.  SDL TEARDOWN GUARDED WITH SDL_WasInit
--------------------------------------------------------------------------------
Three sites: [.SRC]I_VIDEO.C:526 (I_ShutdownGraphics), [.TEXTSCREEN]TXT_SDL.C:297
(TXT_Shutdown), and the ordering note at [.SRC]I_ENDOOM.C:42.

SDL_QuitSubSystem is documented as reference-counted, so a redundant call
"should" be a no-op.  UNDER DECWINDOWS IT IS NOT SAFE TO RELY ON THAT: a second
video teardown reaches Xlib as X_FreeColors against an already-freed colormap,
and Xlib's DEFAULT ERROR HANDLER CALLS exit().  The game would die during its
own exit path - after the user quit, so the symptom is a spurious error at exit
rather than an obvious crash, which makes it exactly the kind of thing that gets
mis-diagnosed.

This is not hypothetical here: there are TWO video-teardown sites reachable from
a single I_Quit() - I_ShutdownGraphics() and TXT_Shutdown() by way of I_Endoom,
which draws the ENDOOM lump as the game exits.  Both are now guarded:

    if (SDL_WasInit(SDL_INIT_VIDEO))
    {
        SDL_QuitSubSystem(SDL_INIT_VIDEO);
    }

Both also NULL the display surface afterwards, since it belongs to the subsystem
just dropped, so nothing can hand a dangling surface back to SDL.

I_ENDOOM.C:42 additionally CHECKS TXT_Init()'s return value, which upstream
ignores.  Every failure path in TXT_Init leaves the text screen unusable, and
the code after it would then draw through a null screendata pointer.  It is also
the return value section 9 makes meaningful.

--------------------------------------------------------------------------------
9.  UNCHECKED ALLOCATIONS
--------------------------------------------------------------------------------
Two mallocs whose results were memset through without a check.

9.1  [.SRC]I_VIDEO.C:1146 - the window icon mask

     The icon is cosmetic, so failure must not take the game down: the function
     now returns early, and the game runs without a window icon.  (The
     surrounding code also has a related hazard noted at i_video.c:1178 -
     SDL_WM_SetIcon dereferences the surface immediately, so the surface must be
     valid at the point of the call.)

9.2  [.TEXTSCREEN]TXT_SDL.C:266 - screendata in TXT_Init

     Here failure IS reportable: TXT_Init already has a documented int return,
     so it now returns 0 and the caller backs out.  That is what makes the
     check added in section 8 meaningful.

Neither is a VMS-specific problem.  They are worth fixing here because this is a
platform where a modest process quota (PGFLQUOTA / VIRTUALPAGECNT) makes a
failing malloc a REALISTIC outcome rather than a theoretical one - and a
null-pointer memset on VMS is an ACCVIO with no useful message.

--------------------------------------------------------------------------------
10.  [.SRC.HEXEN]MN_MENU.C - DUPLICATE DEFINITION OF demoextend
--------------------------------------------------------------------------------
mn_menu.c:134 contained

    boolean demoextend;     // from h2def.h

which is a second DEFINITION of a variable [.SRC.HEXEN]G_GAME.C:100 already
defines and h2def.h:636 correctly declares extern.  The comment shows the intent
was to declare, not define.  Changed to rely on the extern declaration.

This is an upstream bug that survives only on toolchains which quietly merge
duplicate tentative definitions into one - the traditional "common block"
behaviour of GCC before -fno-common became the default.  A strict linker rejects
it outright:

    multiple definition of `demoextend';  g_game.o: first defined here

Worth understanding rather than just silencing: had the two objects each kept
their own storage, h2_main.c's "-demoextend" switch and mn_menu.c's two resets
could write a DIFFERENT variable from the one g_game.c tests, so the switch
would appear to be ignored intermittently.  Fixing the definition is what makes
the four writers and one reader agree.

--------------------------------------------------------------------------------
11.  [.SRC]M_CONFIG.C - WHERE THE .CFG FILES AND SAVEGAMES LIVE
--------------------------------------------------------------------------------
GetDefaultConfigDir() gained a VMS branch (m_config.c:2110) that returns "",
i.e. the CURRENT directory - the same behaviour as the existing Windows path.
DEFAULT.CFG, CHOCOLATE-DOOM.CFG and the savegames therefore live in the
directory the game is run from, normally the one holding the IWAD.

Not the ~/.chocolate-doom/ scheme, for three reasons:

  1. It would create a directory whose name begins with a dot and contains a
     hyphen.  On an ODS-2 volume that is not a legal directory name, and even on
     ODS-5 it needs escaping.
  2. $HOME is not normally defined in a DCL session, so getenv("HOME") returns
     NULL and the function would fall through to "" anyway - just without
     saying why.
  3. It matches the sibling OpenVMS ports (Hexen, Chocolate Duke, Kyra), which
     are all run from inside the game data directory.

Returning "" is a SUPPORTED upstream mode, not a special case: M_GetSaveGameDir()
and M_SetConfigDir() already test for it, and it also suppresses the
M_MakeDirectory() calls, so nothing tries to create a directory on VMS at all.

Note this is guarded by #if defined(VMS) || defined(__VMS) - the one place in
the port that tests for the platform - so the Linux pre-validation build keeps
upstream's behaviour.

--------------------------------------------------------------------------------
12.  WHAT WAS NOT CHANGED, AND WHY
--------------------------------------------------------------------------------
Recorded because each of these looks like an omission:

  * QUOTED INCLUDES.  Not one needed rewriting.  DEC C maps a quoted include
    containing a slash onto an RMS filespec and cannot stack a relative base
    directory, so "foo/bar.h" and "../bar.h" both fail with %CC-F-NOINCLFILEF -
    the sibling Duke port had to rewrite 17 of them across 11 files.  Chocolate
    Doom's sources contain ZERO slashed or parent-relative quoted includes,
    verified across all four build sets.  Keep it that way.

  * HEADER TYPE SELF-SUFFICIENCY.  A transitive include-closure sweep found no
    header using intN_t before something provides it, so the sibling Duke port's
    %CC-E-TYPEDEFNOTDEF class of failure does not arise here.

  * NO /POINTER_SIZE, and no pointer-size-related edits.  Unlike Duke - whose
    Build engine stuffs pointers into int32_t and therefore must stay 32-bit -
    Doom keeps pointers in pointers.  The two deliberate pointer/integer
    aliasing spots are both properly typed: the thinker function_t union in
    [.SRC.DOOM]P_TICK.C is a union of function POINTERS, and Hexen's
    specialval_t (h2def.h:175) stores its integer as intptr_t, which is why
    sv_save.c can round-trip a pointer through special1.

  * IMPLICIT FUNCTION DECLARATIONS.  There are none; every function is declared
    before use (checked with -Wimplicit-function-declaration over all four build
    sets).  So no header was added for this, and DESCRIP.MMS's warning-disable
    list has no tag for it.

  * THE 33 INCREMENT SITES.  Section 6.8.

  * [.SRC]Z_ZONE.C vs Z_NATIVE.C.  The port compiles Z_NATIVE.C; the reasoning
    is in DESCRIP.MMS.  Neither file contains a crash idiom, so this is a
    platform-fit choice, not a compiler fix.

  * [.SRC]AES_PRNG.C, ICON.C, W_FILE_POSIX.C, W_FILE_WIN32.C and the
    SDL_mixer/SDL_net files are not compiled at all.  See DESCRIP.MMS for each.

================================================================================
