fmt_compile.c: Alter FINDCOMP(): the caller must supply the semicolon.

The macro expands to a for-loop, including the semicolon body.  The
caller typically supplies their own semicolon.  This results in two on
one line that looks odd.  Alter the macro to have the for-loop inside a
do-while loop that doesn't have a semicolon, ensuring the caller must
supply it.

argsplit(): Incorrect type used in sizeof calculation for array.

Switched to referring to the variable being assigned the resulting
memory rather than duplicating its type information.  Fixes 78211e93.

get_file_info(): Flip logic throughout, reducing nesting.

Now a series of small paragraphs, each of which might return on error.
Also, the Cygwin oddity described in the comment needs no special
handling given the recent bug fixes so it can be deleted.

get_file_info(): Don't return filename from quote onwards on error.

If `file_name' contained a quote then `cp' points to it, and remains
pointing to it if any of concat(), popen(3), or fgets(3) fail to give
the expected result.  On returning, cp is not NULL and so strdup(cp) is
returned.  Also, if fgets() fails on Cygwin in the way the comment
describes then cp is wrongly free(3)'d.  (One of the problems with
generic variable names is their overloaded semantics cause faulty
re-use.)  Fixes b4f2851d and 0c50c669.

get_file_info(): Don't dereference out of scope char array.

A pointer to `char buf[...]' is taken, the array being declared in a
block scope inside the function.  The pointer is passed to strdup(3) as
the function returned, but the array is by then out of scope.  Move the
array to the function's scope.  Fixes 0c50c669.

Replace parallel MIME-preference arrays with struct array.

New array of struct has `type' and `subtype' members, replacing the old
preferred_types[] and preferred_subtypes[] arrays.

pidstatus(): Rewrite to print stopped and continued statuses too.

Delete own versions of WTERMSIG and WCOREDUMP macros now we're assuming
POSIX.

pidstatus(): Delete commented-out code testing for exit(255).

Pre-dating git, a test of wait(2)'s status having its top eight bits all
set was commented out an "I've no idea what this does" explanation.  The
top eight bits hold either exit(3)'s parameter or the signal that caused
the child to stop.  Either way, there are macros for pulling apart that
value and they're being used.  Perhaps they were broken on a much older
system.  Delete the comment, including its code.

Replace printf("%s\n", foo) and similar with puts(foo).

datetime.c: Flip if-conditions so returns move from else block.

Then there's no need for else-blocks after return.

icalendar.l: Refer to base64.h relative to root of source.

Ken reported a missing-include-file problem when building so the product
ends up outside the source tree, e.g. running configure from elsewhere.
It must also depend on his environment, e.g. OS or compiler, because the
problem isn't reproducible with Arch Linux and gcc, and be specific to C
source built from other source files, e.g. lex(1).  However, given that
the existing sibling icalparse.h is referred to as sbr/..., duplicate
that so it builds for Ken too.

test/getcanon.c: exit(3) with 0 or 1, not -1 or symbolic error code.

gethostname(3) returns -1 on failure and that was being returned from
main;  use 1 instead.  getaddrinfo(3) returns EAI_ADDRFAMILY et al on
failure, but here they're defined as small negative integers, unsuitable
for main()'s return value;  again, use 1 instead.  Along the way, flip
the logic so the end is nigh when possible, avoiding the cognitive
overhead of tracking state when continuing to read through the source.

test/getfullname.c: Move return case first in if-else chain.

Refer to #include files from the root of nmh's source.

One of the -I options given to the C compiler is the root of the nmh
source.  This means uip/foo.c's #include of the relative ../sbr/bar.h
can also be written more tidily as sbr/bar.h.

icalparse.y: Remove else-block that returns by merging if-conditions.

The code tested two values to see if they were present, and if they
were, and were equal, then it returned this node.  If either were
missing then this node was still returned because it matched the more
limited criteria that had already been tested.  So the test for
returning this node can more simply be if either value to compare is
missing, or they're (both present and) equal.

seq_setprev.c: Flip if-condition so return moves from else block.

Then no need for else block after return.

folder_pack.c: Flip if-condition so return moves from else block.

Then no need for else block after return.

seq_setunseen.c: Flip if-condition so return moves from else block.

Then no need for else block after return.

mhstoresbr.c: Flip if-condition so return moves from else block.

Then no need for else block after return.

read_switch_multiword.c: Flip if-condition so return moves from else block.

Then no need for else block after return.

datetime.c: Don't atoi("0") if zone is NULL.

datetime.c: Fix parse-failure's error-message's daylight indication.

`in_standard' was being used to decide between `standard' and `daylight'
in the error message, but it would always be false by that point.
`in_daylight' is what's passed to the parsing function so use that to
determine the string instead.  Bug present since the initial
implementation.

datetime.c: Replace some int with bool.

Only call fork(2) once, not up to five times on failure.

The existing loop that slept five seconds between each attempt was
copy-pasted widely.  Those days are gone.  Just call it once.

content_error(): Add missing va_end(3).

A va_start(3) without a matching va_end() is undefined behaviour.

Send a QUIT instead of RSET at session end when doing 'whom'.

When running 'whom -check' (which really invokes post(8)), at the
end of the SMTP session we would send a RSET instead of a QUIT.  This was
technically a RFC violation (RFC 5321 says a QUIT has to be the last
thing you send), and this would cause some SMTP servers to complain.
So make sure if we're being invoked by whom to send a QUIT at the end
of the session.  Reported by Ralph Corderoy.

Add a -credentials argument when we call post.

If we are using -check, post(8) will need to talk to a remote SMTP server,
and it might need to perform authentication when doing so.  So include a
-credentials option if there is the appropriate line in the user's profile.

popsbr.c: Remove impossible case from switch statement.

Actually, just convert the overkill switch statement into a couple of
if-statements as it's a lot less lines.

mhbuildsbr.c: Attempt fork(2) multiple times, as intended.

The loop attempting fork() intends to try five times but the comparison
operator was wrong so would only try once.  Bug present since pre-git.

base64.c: Don't out-of-bounds printf("%s", &encoded[-1]).

`cp' is walking through encoded[] when an error occurs and is stepped
back up to 20 elements to provide some lead-in context for the error
message.  If might be stepped back to encoded-1, but it attempts to cope
with that by `cp ? cp : encoded'.  cp is always non-NULL so true and cp
is printed.  Presumably, `cp > encoded' was meant.  But it's all a bit
of a rigmarole so just use min() instead to ensure cp stays within
encoded and print cp.  Fixes bfc6b93af.

content_error(): Don't strlen(invo_name) that might be NULL.

Earlier in content_error(), it checks invo_name isn't NULL before using
it.  Later, it passes it to strlen(3) without the check.  Either the
former is redundant or the latter wrong.  Rather than work out which,
delete the strlen(invo_name) because it was only used for a
variable-width indent on a second diagnostic line.  Instead, indent by a
fixed four spaces, which will look better when invo_name is stupidly
long anyway.  Bug present since pre-git.

replsbr.c: In parent, free child's argv[] built before fork().

Fixes ccafa1944.

getpass.c: Don't fileno(NULL) when fopen("/dev/tty") fails.

If stdin is a TTY, but opening it for writing fails, then stdin and
stderr are used as defaults and the TTY doesn't have echo disabled.  The
later test to restore echo just checks if stdin is a TTY and not that it
was opened successfully causing a NULL FILE pointer to be fileno(3)'d.
Fixes da6af9633.  This function's logic remains a bit contorted;  I went
for the minimal fix.

mhbuildsbr.c: Don't free(3) string then print it; do reverse.

Fixes c87ca5cca.

smtp.c: Use read-end of pipe, not random integer.

Instead of `pdi[0]' the array of two elements was being indexed by `i'.
That is typically zero so everything works, but can be up to five
depending how many times fork(2) failed before succeeding.  Fixes
e65127948.

Revised expected output with -prefer in test/mhlist/test-mhlist.

Update to commit 5dce1b38ace6cf61398df3ee4ca5aee0e1dedb5d.

man: Fix some of the font-changing macros' parameters.

Various recurring problems on two themes.  `.BR foo,' lacks the space in
`.BR foo ,'.  `.IR foo' doesn't need to alternate and can be just `.I
foo'.  Other similar changes made, e.g. `.B foo' becoming `foo'.

Add -noprefer switch to mhshow, mhstore, and mhlist

Allow profile-specified -prefer switches to be cancelled from the
command line.

Change the priority ordering for multiple -prefer switches

Interpreting multiple -prefer switches in ascending priority
order allows the command line to override the user's profile.

man: Use `.IR' for `foo(1)', not the few `.BR'.

man: Change the tautological `the foo(1) man page' to `foo(1)'.

oauth: Replace `if...return false; return true' with `return ...'.

In functions returning bool, these are equivalent.

    Before:                 After:
        if (!foo())             return foo();
            return false;
        return true;

oauth.c: Remove unused assignments of function return value.

`if ((status = foo()) != OK) return false;' doesn't need to set local
variable status.

Replace FALSE and TRUE with C99's false and true.

Remove the duplicate definitions of FALSE and TRUE.  Change a few
variables from int to bool at the same time.  The semantics of bool are
different to int because `bool b = 42' maps 42 to 1, but if the code was
relying on that then it needs shaking out anyway.

print_intro(): Change parameter brief from int to bool.

flist.c: Use bool instead of int for some of the flags.

Replace boolean with bool everywhere.

boolean's comment said it existed to ensure storage was a char and not
an int so it could be packed in a struct, but the only struct using it
doesn't care about the space taken.

mh.h: Always define NMH_UNUSED as a cast to void.

The not-gcc-or-earlier-than-gcc-2 version was the identity function
seems unnecessary as what C99 compiler would complain about a cast to
void?

mh.h: Move the gcc-4.3-conditional macro definitions.

Having them outside of the gcc-2 ones makes the logic easier to follow
as there's no nesting.

dropsbr.c: Remove unused ntohl() macro, and its NTOHLSWAP test.

Not used since e6c91771 removed support for ./.foo.map index for ./foo
mailbox.

fakehttp.c: Remove unused LINESIZE macro; never used.

config.c: Remove unused nmhdocdir() macro.

ap.c, dp.c: exit(3) with 0 or 1, not [0, MAX_EXIT].

Don't attempt to indicate the number of failures through the exit status
as that's too constrained.  Just stick to zero for none, and 1 for some.
Delete, the now unused, MAX_EXIT.

DEFAULT_PAGER: Move from nmh.h to mh.h.  Former is "config.h".

The macro isn't related to platform independence, unlike the rest of
nmh.h, that starts by including config.h.

geteditor.c: Hard-code the default, "vi".

It was the only use of the DEFAULT_EDITOR macro, so that can be deleted.

Move base64.c's prototypes from prototypes.h to new base64.h.

mhbuild.man: Fix grammar indentation by expanding tabs.

The grammar is pre-formatted using spaces for indentation.
A few lines had switched to tabs in 9b706433f.

decodeBase64(): Limit length of error message on invalid input.

Screenfuls of email were appearing on stderr.

Restrict use of alloc_size function attribute to gcc >= 4.3.0.

It's not supported by gcc 4.2.4.  CHECK_PRINTF doesn't work
perfectly with gcc 4.2.1, so disable that with gcc < 4.3.0 as
well. Fix to commit c066b395274021182a5f0530ccfeb1bcd167d860.

Move read_line.c's prototypes from prototypes.h to new read_line.h.

Add copyright comment to start of read_line.c whilst there.

Add ENDNULL macro for gcc's `sentinel' function attribute.

Annotate a few prototypes with it.

Move terminal.c's prototypes from prototypes.h to new terminal.h.

Fix spelling in comment.

read_switch_multiword.c: siglongjmp(3) removes need for `else'.

mhlsbr.c: Don't need to `else' after longjmp() at end of then-block.

Remove comment asking if siglongjmp(3) should be used;  it is.

77a8a873b switched from longjmp(3).

Don't need to `else' after mhladios() at end of then-block.

Give mhldone() and mhladios() the NORETURN function attribute.

And remove `FALLTHRU' comments after their calls.

Don't cast zero to pid_t for setpgid(2)'s first argument.

Don't cast zero to char for storing at pointer to char.

Don't cast zero to a non-function pointer;  use NULL instead.

Better still, omit it when BSS does the job, or C's treatment of
non-zero as true is sufficient.

mhparse: Change mhfixmsg flags from int to bool.

mhparse.h: Add externs for skip_mp_cte_check, etc.

Delete the local copies from the other user.

Delete declaration already available from included header file.

mhparse.h: Add externs for preferred_types[], etc.

Delete the local copies from other users.

Delete declarations already available from included header files.

mhbuildsbr.c: Include mhcachesbr.h.  Delete copy of prototype.

mhfixmsg.c: Make mhfixmsgsbr() static.

Move NPARTS and NTYPES from mhparse.h to mhmisc.h.

It's mhmisc.c that has the global arrays that use them.

mhmisc.h: Add mhmisc.c's global variables as externs.

Delete the local copies from other users.

Create mhmisc.h with mhmisc.c's global-function prototypes.

Delete the many local copies from each caller.

Make functions with no external callers by name static.

rcvtty.c: Remove duplicate prototype.  Other in mts.h.

aliasbr.c: Remove duplicate prototypes.  Others in aliasbr.h.

mhparam: exit(3) zero if all components found, else one.

The exit status used to be a count of the number missing, clipped to
120.  That doesn't seem useful, and is overhead to document, read, and
test.  Use the normal Unix 0 or 1 instead.

Ensure that mhfixmsg exit status is either 0 or 1.

Ralph noticed where exit status values were being accumulated, and those
values could include -1.

Add gcc's pure function attribute.

Mark quite a few function prototypes with it.

inc.c: Make maildir_srt() static.

Replace add(foo, NULL) with mh_xstrdup(foo).

add()'s arguments are back to front so add(foo, bar) produces bar+foo in
the normal case.  Thus add(foo, NULL) is read as the jarring NULL+foo.
Removing the NULL with mh_xstrdup() avoids this.  FENDNULL is used when
it isn't obvious foo can't be NULL as add() treats it as "" in that
case.

Replace strlen("foo") with LEN("foo").

The existing LEN() uses sizeof.

context_find.c: Hoist strlen(3) out of search loop.

Add die(fmt, ...).  Equivalent to adios(NULL, fmt, ...).

Avoids the noise of the `NULL' first parameter that's used in over 70%
of adios() calls.  Removes the possibility of it being omitted and `fmt'
being used instead.  `die' is already in use in nmh's shell scripts.
Had to rename post.c's existing die() to avoid it clashing.

mhparam: Remove `libdir', deprecated in 1.7.

Use existing `libexecdir' instead.

pick: Remove support for MHPDEBUG environment variable.

It was deprecated in 1.7, and the -debug option gives the same
behaviour.

Add MAX_EXIT, value 120, to limit some more exit(3) values.

mhparam(1) altered to use new macro.  ap(1) and dp(1) now limit.

mh_xmalloc() et al: Use C99's `%zu' to print size_t.

Treat %(myhost) failure to match canonical hostname as information.

Not an error from test-myhost.

Replace printf("foo\n") with puts("foo").

Add gcc's nonnull function attribute.

Apply it to some prototypes to get started.

Use C99's variable-argument preprocessor macros for ALLOC_SIZE.

ALLOC_SIZE2() can be deleted.