Dear Maintainer,

sail(6) cannot be played on any architecture where plain "char" is unsigned.
I found this on arm64 (Raspberry Pi 5, Debian 13.7); by the same reasoning it
affects armel, armhf, ppc64el, riscv64 and s390x, and not amd64, i386,
loong64 or mips64el.

What happens

Start "sail", take scenario 0 (Ranger vs. Drake) and either ship, give a
move such as "m" "3" and wait a few turns.  On amd64 the Drake closes and the
two sloops fight within half a dozen turns.  On arm64:

  * the other ship disappears from the view on the first turn and is never
    seen again -- I played 278 turns once, moving towards her all the time,
    while she got further away;
  * the wind vane at the right-hand edge of the view is wrong from the first
    screen: the wind speed digit is there, but the '+' is missing and the tail
    ('|', '/', '-' or '\') sits two rows lower and one column to the left of
    where it belongs;
  * when broadsides do bear (ships can still meet if they only ever move
    north or west), every one of them does the maximum damage in the tables.

Why

sail/globals.c keeps small signed numbers in plain char:

    const char dr[] = { 0, 1, 1, 0, -1, -1, -1, 0, 1 };
    const char dc[] = { 0, 0, -1, -1, -1, 0, 1, 1, 1 };

and likewise the gunnery modifier tables AMMO[], HDT[], HDTrake[] and QUAL[],
which hold values from -1 to -4.  With unsigned char each -1 is 255, so

    sp->file->row -= dr[sp->file->dir] * dist;        (step(), dr_3.c)

moves a ship heading south by -255 rows instead of +1; any heading with a
southerly or easterly component, and drifting before a northerly or westerly
wind, does the same.  Sterns are computed 255 squares from their bows, so
range(), gunsbear() and portside() in misc.c are wrong as well.  In pl_7.c
draw_slot() does wmove(slot_w, 7 - dr[winddir], ...), which fails for row
-248, so the vane's tail is drawn where the cursor happened to be and the
mvwaddch() of the '+' fails outright.  In dr_1.c and pl_3.c the hit value
"HDT[..] + QUAL[..] + AMMO[..]" comes out around +250 instead of slightly
negative, passes the "hit >= 0" test and is clamped to the top row of the
damage tables (grape shot is not clamped at all).

The attached chartest.c shows the arithmetic in ten lines; on arm64 it prints

    plain char is unsigned: dr[5] = 255, the ship is now in row -248 (should be 8)

A second instance of the same assumption is in sail/sync.c:

    switch (*p++ = getc(sync_fp)) {
    ...
    case EOF:

The switch is on a char, so with unsigned char "case EOF" can never match
(gcc: "case label value is less than minimum value for type"), and a string
record that is cut short at the end of the synchronization file makes every
player and the driver loop for ever with the file locked.  NetBSD has since
fixed this one, but its current sail still has the "const char" tables, so
the main bug is present upstream as well (hence the "upstream" tag).

The fix

The attached patch (DEP-3 header, applies with -p1 after the 22 patches in
2.17-35, as debian/patches/0023-...) declares the six tables "signed char"
in globals.c and extern.h, and reads the synchronization file through an
int.  It changes nothing on architectures where char is signed.

I went through the other plain-char members of sail's structures (struct
File, shipspecs, scenario, windeffects, Tables): none of them ever holds a
negative value (the -1 "no fourth mast" marker is in rig4, which is an int),
so the six tables and the getc() are all there is.

Testing

I built sail from the 2.17-35 source on arm64 with and without the patch,
with gcc's default (unsigned) char in both cases.  Without it: the vane as
described, and by turn 4 neither ship was in the view any more.  With it: the
vane is drawn correctly ('+', speed, tail in one line) and by turn 3 the Drake
had closed with the Ranger, collided and opened fire.  Building the unpatched
source with -fsigned-char gives the same result as the patch, which would be
a one-line alternative (for sail only) if you prefer not to touch the source.

I have also been playing, and testing under gcc's address and undefined
behaviour sanitizers, a build with this fix (and a good many other changes
that are not part of this report) without seeing any other signedness
problem.

Dear Maintainer,

after reporting sail I went through the rest of the package for the same
assumption.  Two more games are affected; a second patch is attached
(0024-..., DEP-3, applies with -p1 on top of the sail patch or without it).

adventure (bsdgames-adventure): end of input is never noticed.

    $ printf 'no\n' | /usr/games/bsdgames-adventure | head -c 100000 | grep -c 'Give me a break'
    2479

  getin() in adventure/io.c stores getchar() in a char and then has "case EOF:"
  in a switch on that char.  With unsigned char the case is unreachable, so on
  ^D, or when piped input runs out, the game prints "Give me a break!!" and
  "I don't know that word." for ever (at full speed) instead of "user closed
  input stream, quitting...".  The same source built with -fsigned-char prints
  that message and exits 0.

trek: a lost game gets the bonus for winning.

  Start trek, take a short novice game, and "destruct".  On arm64 the score is

    Penalty for 5 klingons remaining         -2000
    Bonus for winning a novice game            100
    Penalty for getting killed                -500

  Move.endgame (trek/trek.h) is a plain char; lose() sets it to -1, win() to 1,
  and score() tests "Move.endgame > 0".  255 > 0.  Built with -fsigned-char the
  bonus line is absent and the total is 100 lower.

How I looked, so that you can judge what may have been missed:

  * every game built with gcc 14 and -funsigned-char -Wtype-limits
    -Wswitch-outside-range: the only relevant diagnostics in the whole tree
    are sail/sync.c:260 and adventure/io.c:88 (both "case EOF" on a char);
    no comparison of a plain char with a negative constant or with EOF exists
    anywhere else;
  * every source file through -fsyntax-only -funsigned-char -Wsign-conversion,
    looking for negative constants converted to char: 93 in sail/globals.c
    (the tables of the first patch) and one in trek/lose.c (endgame = -1);
  * hack keeps its small signed numbers in "schar", which config.h already
    defines as short int for exactly this reason;
  * the 230-odd remaining scalar plain-char variables and the char arrays
    that are assigned differences at run time were read through: letters,
    symbols, flags and counts that never go negative;
  * adventure and trek were then built twice, with -fsigned-char and with
    -funsigned-char, and behave differently exactly as described; with the
    patch the unsigned build behaves like the signed one.

Not related to signedness, but seen on the way: several games loop at end of
input whatever the architecture (monop asks "How many players?" for ever,
atc, backgammon, teachgammon and snake likewise fill the terminal).