msgchk.c: Remove UUCP tests; unused since `#ifdef MF'.

The values UUCPOLD et al are never set so don't bother testing for them.
They haven't be possible in git's history.  They were guarded in the
past by `#ifdef MF';  that stood for `mail filter'.

Replace getcpy(f()) calls with mh_xstrdup() when f() can't be NULL.

And comment a few functions as never returning NULL along the way.

Delete "blank" lines at start of files to bring content into view.

utils.c: Improve comment describing add().  Suggest mh_xstrdup().

imaptest.c: Don't use error message as adios()'s format.

Add a `"%s"' format string.  Stops gcc complaining.

imaptest.c: Don't test possibly uninitialised local variable.

get_imap_response() doesn't necessarily set the content of a non-NULL
`char **status'.  Do this before calling, and check if it's been set to
non-NULL before using upon return.

imaptest.c: Don't free brkstring()'s value; it's re-used.

Subsequent calls to brkstring() write to free'd memory, corrupting
malloc()'s internal data.

imaptest.c: Add `format(printf, ...)' attribute to functions.

Stops warnings from gcc for send_imap_command() and add_msg().

Fix invalid pointer arithmetic.

Reported by David Levine.

Support command issuance and timestamps.

Still more fiddling.

Add netsec_get_sasl_ssf().

Add the function netsec_get_sasl_ssf(), which retrieves the SSF
(strength security factor) negotiated as part of the SASL exchange.

A little closer to being somewhat useful.

Make snoop work like it used to for SMTP.

Fix up some const and unsigned char warnings.

Improved, a bit.  Slightly more useful.

Rework snoop support slightly for the new world order.

Make sure we do base64 snoop decoding at flush time.  Still need to
fix SMTP support.

Rototill snoop support so it works at flush time.

Before on writes, the output snoop support would only work when it was
doing netsec_vprintf().  Do it at flush time, so it should work for
any type of write.

It does just what it says on the tin.

You should not use this unless you're the sort of person who regularly
reads the commit log. Still incomplete for now.

Add context information to SASL callback.

Provide an extra context argument for the SASL negotiation callback
that application code can use to pass down information that the
callback might need.

Switch to the use of to_upper() instead of custom code.

Improve this documentation a bit.

Remove mmdf_magic and empty_line temp files on success.

Fixes commit b46d49ba.

picksbr.c: fprintf(3) function pointer using `%p' and union.

Using `%#llx' and cast to `unsigned long long' caused Cygwin woes.
Fixes 07916b2ff.

slocal.c: Use C99's `%td' to printf(3) ptrdiff_t values.

Using `%ld' caused Cygwin woes.  Fixes 6015bb1f7.

inc/test-eom-align: Cut down on the flicker by re-ordering tests.

inc/test-eom-align: Replace case on $MBOX_TYPE with interpolation.

inc/test-eom-align: rm(1) unwanted +inbox emails, not rmm(1).

Using rmm(1) is using the software under test.  And about 50 times
slower.

inc/test-eom-align: Create test mboxes in less steps.

Use a couple of temporary files created at the beginning to allow runs
of alternating cat(1) and echo(1) to be a single cat.  Knocks 15% off
the `make check' time for that single test in a simple trial here.

inc/test-eom-align: Fix MMDF-mbox `B' size calculation.

Also, have the email body end in what appears to be the MMDF magic
string for the first few bytes;  otherwise, the test mbox is just a
boring single email compared to the `A', From_, version.  Fixes
8699f1cc.

inc/test-eom-align: Fix MMDF-mbox `A' size calculation.

The `...LINESZ' variables are number of bytes, not number of lines, so
MMDF's is now 5 rather than 1.  And the MMDF magic string tops and tails
each email so needs to be subtracted twice, not once like From_'s.
Fixes 8699f1cc.

test/common.sh.in: Only rm(1) if the cd(1) succeeds.

The rm specified the full path anyway, but if the cd fails for some odd
reason then it's best not to continue.

annosbr.c: free(3) takes void pointer; don't cast non-const pointer.

(const pointers are another whole can of worms, and their const
poisoning causes much noise for little benefit.)

void-pointer parameters don't need casting to char pointer.

E.g. `memcmp((char *)digest...' doesn't need the cast if it's already a
pointer.

Replace `sizeof (char)' with 1, then remove if a multiplier.

Don't cast void pointer return value of malloc() et al.

Stop failed m_mktemp2() that give directory reporting get_temp_dir().

The first parameter of m_mktemp2(), if not NULL, is a path and the
basename is used as the directory to hold the new temporary file, but
the error message on failing to create that used get_temp_dir() and
that's often somewhere else, e.g. `mhbuild /dev/null' attempts /dev/...
Don't attempt to state the failed path in the error message as at least
that's not misleading.  Perhaps the function needs some other indication
of success so it can pass back the attempted filename that failed.

mhbuild.c: Don't use set_done() to unlink temporary files.

Because they're created with m_mktemp2(), the temporary files are
already in a list to be unlink(2)'d at exit(3), thanks to atexit(3), so
don't set_done() to attempt that twice.  Updates 6bc64765f.

mhshow/test-charset: Test iconv(1)'s result, not just its exit status.

Problems with generating the input to iconv(1) were hidden as long as it
could be converted from EBCDIC-US to UTF-8 because only the exit status
was tested.  Check the output is as expected too.

mhshow/test-charset: printf(1)'s octal doesn't need a leading zero.

The `\0344' was being interpreted as `\034' and `4'.  This can still be
converted from EBCDIC-US to UTF-8 by iconv(1) so the test worked, but it
was misleading.  Remove the leading zero as printf's octal escape is
`\nnn' where nnn is one to three octal digits.  It's echo(1) that
insists on a leading zero.  Fixes 6fd1d18f.

inc/test-eom-align: Use octal, not hex, in printf(1) format string.

POSIX's printf(1), and dash(1)'s, does not support hex.  Fixes 8699f1cc.

mhshow/test-charset: Use octal, not hex, in printf(1) format string.

POSIX's printf(1), and dash(1)'s, does not support hex.  Fixes 792d7e1e.

Added tests of MMDF mailbox format to test-eom-align.

runpty.c: Move child slave code into new run_command().

runpty.c: Handle fork(2) failure first, rather than child.

Gets the error handling out of the way so it can be forgotten.

runpty.c: Factor duplicate code into new open_master_pty().

Only called twice, but it shows the same actions are done both times.

runpty.c: Replace fprintf()+exit() pairs with new die().

Detect function-pointer `done' being set twice in a row.

Make `done' a function that uses a file-static function pointer, and add
set_done() to alter it.  That wants either the old or new value to be
exit(3).  If it's not, it still alters the stored pointer, to maintain
the existing behaviour, but emits a warning on stderr.  mhbuild(1)
triggers that warning, making this hunch worthwhile.

man: Replace csh `%' prompt in examples with sh's `$'.

Youngsters these days probably wouldn't recognise csh.  None of the
commands needed changing due to syntax differences.

mh-draft.man: Use ASCII double-quote for itself in example output.

Not the TeX doubling of backquote and single-quote.

man: Add backslash before backquote so it appears as sh backtick.

man: Fix backquote used in error for single quote.

A repeated bit of prose had the typo `foo` instead of `foo'.

mhlsbr.c: Delete file-static function pointer mhl_action.

Never set, so always NULL due to BSS.  It used to be set by c808c5d5.

mhlsbr.c: Delete `jmp_buf mhlenv', never setjmp(3)'d.

Only ever conditionally longjmp(3)'d.  The setjmp() was removed by
c808c5d5.

Strip parenthesis from `return (state = OK);' and similar.

There are sufficient cases without the parenthesis that various
compilers must be happy without.

pop_init(): After proxy's execve(2) failure, exit(1), not exit(10).

The value 10 seems undocumented, and I can't find anything that tests
for that specific value.

sendfiles: Always die with exit status of 1 rather than -1.

The sh function die() took the exit status as an optional parameter,
with a default of -1, but this was never used so remove the option and
always use an exit status of 1.

Remove preprocessor conditionals on `lint'.

Treat all tests for `lint' being defined as false.  Don't know when
anyone had a lint(1) that could be run on the source, but none of the
areas had been touched in git's history, and at least one would have
been a compile error had it been defined.

Replace `_exit(-1)' with `_exit(1)'.

In line with other changes to limit a process's exit value to small
non-negative integers distinguishable with sh's `$?'.

whom.c: Don't call rename(2) with source that's uninitialised.

The intent is that two rename(2)s take place: to move a file to a
backup and then return it.  The first test controlled the generation of
the destination in the char array `backup' and calling the first
rename().  The second test used that same array for the restoring
rename(), but used a different test and so could run when the first
rename hadn't, and with an uninitialised array for the source filename.

The initialisation of `backup' and the first rename needed both
environment variables `mhdist' and `mhaltmsg' to be non-empty, and the
former to have a non-zero atoi(3) value.  The second rename ignored
`mhaltmsg'.  Bug pre-dates git.

whom.c: Re-work fork/exec logic to clarify interaction with distsw.

If fork(2) fails then die;  don't fall through to execve(2).

It's already been agreed that failure to fork() is so rare these days
that multiple attempts aren't required.  Nor should the code persevere
by calling execve() as if the fork had succeeded, else why bother to
fork?  Instead, die.

whom.c: Don't increment atoi(3)'s return value as it's already true.

atoi(3) has returned non-zero and distsw holds that value.  Only the
truthness of distsw matters.  Don't increment distsw, as if to show it's
been set, because that would turn the true -1 to false 0.  Bug pre-dates
git.

whom.c: Don't store atoi(getenv("mhdist")) when value only tested.

It's the non-zero value of atoi(3) that's important in the if-condition;
the value is never used again.

mhlsbr.c: Don't strchr(3) non-string NUL-less buffer.

Up to 8 KiB was read into a buffer and then strchr() used to test for a
linefeed.  There was no guarantee the linefeed would be present, else
why test for it, nor that the buffer would contain a NUL to terminate
the search, either from the read bytes, or the bytes not trampled by
read().  Replace the two similar lumps of code with a new
linefeed_typed().  Bug pre-dates git.

mhlsbr.c: Don't read(2) from fileno(3) of stdout.

Remove the assumption that file descriptor 1 is readable.  It can be if
it, or where it was dup(2)'d from, was opened read/write, e.g. on
/dev/tty.  But it can easily be arranged with shell re-direction that it
isn't, and then the read(2) fails.  Pass 0, standard input's file
descriptor, instead.  Don't use fileno(stdin) as I can't think of a need
to support that being non-zero, but its use makes the reader think it
might.  Bug pre-dates git.

Invert ternary condition to remove one negative.

Replaces `i != NOTOK ? OK : NOTOK' with `i == NOTOK ? NOTOK : OK'.

mime_type.c: Fix regexp in comment that describes following code.

Fixes e35fb433.

addrsbr.c: Alter ismymbox() to return bool, not int.

It was returning 0 or 1.

smtp.c: Have function returning char pointer return NULL, not 0.

ext_hook(): Flip if-statement condition so then-block simply returns.

No need for rest of function to be in an else-block.

new.c: Order two return statements to match comment.

No change in behaviour.

Edit out a few more tautological `the foo(5) man page'.

nmh_completion(): Simplify, removing local variable.

Remove casts of NULL to a data pointer.

With a function prototype in place stating a parameter is a foo pointer,
where foo is data, not a function, then `NULL' suffices so the cast to
foo pointer is redundant.  If the function parameter is retrieved with
va_arg(3) then the pointer passed must be the retrieved type, e.g.
concat()'s arguments are fetched as char pointer and so it should be
called as `concat("foo", (char *)0)'.  Using NULL is incorrect, though
NULL could be used instead of 0 but still needs casting.  However, the
source doesn't bother getting this right and just passes NULL in most
cases so make the few match.  Most of those changed were passing NULL
cast to a void pointer.

Remove more outer wrapping parenthesis from return statements.

For example, `return(foo(a, b));' becomes `return foo(a, b);'.

dmktime(): Break assignments so `*=' and `+=' can be used.

Add comments tracking the units of `result' from days to seconds.

Replace `return (0)' and similar with `return 0'.

Just outer parenthesis removed.

Hoick FENDNULL(key) out of the search loop.

`key' is a loop-invariant so can be tested for NULL once before the
loop.  In one case, this also avoid passing NULL to printf(3) for "%s",
and means getcpy()s can be replaced with mh_xstrdup()s.

mh_oauth_cred_fn()'s return value no longer needs duplicating.

It now returns a fresh allocation so that can be used directly rather
than obtaining another copy.

mh_oauth_cred_fn(): Always freshly allocate result.

If a `credential-file' context was found, and started with a `/', then
the m_defs's n_field was returned, otherwise a newly allocated string.
The caller couldn't tell whether to free the result or not so memory was
leaked.  Alter it to always freshly allocate the result.  Fixes
803f25412.

exmaildir(): Don't leak memory allocated by path().

Bug pre-dates git.

mime_type(): Don't leak memory allocated by get_file_info().

Fixes b4f2851d4.

lock_file.c: close(2) file descriptor on failure, avoiding leak.

Fixes d6e398f9c.

mhfixmsg.c: Fix memory leak of temporary filename if write fails.

mhfixmsg.c: Fix memory leaks of boundary string on early returns.

Delete mh_xfree(), changing callers to call free(3) instead.

A free(3) that didn't handle NULL is now long past.

rmf.c: free(3) return value from m_mailpath() calls.

And add comment to m_mailpath() that its return value needs freeing.

path.c: Add comments declaring return values must be free(3)'d.

Saves working it out afresh each visit.

path.c: Remove path()'s test for expath() returning NULL.

All paths out of expath() have it calling mh_xstrdup().

path.c: Re-format path() to make it more clear what's happening.

slocal.c: Alter trim() to return static array, not malloc(3).

The callers are immediately passing the return value to printf(3) for
"%s".  There's only one call per printf().  None of the callers are
bothering to free(3) the existing return value.  Return the address of
trim()'s char array, now static, instead.  Leave trim() returning NULL
when passed NULL, even though that gives NULL to print(3) for "%s".  It,
and slocal's other bugs, remain.

Remove test for `\0' in `*s && isfoo(*s)' for ctype.h calls.

find_cache(): Remove test that's always false.

The if-statement's condition is a conjunctive that tests `!writing' and
then, only if that's true, calls find_cache_aux() and tests `writing' in
determining the first argument;  that must be false.  Bug pre-dates git
history.

readline(3) wrapper: Move free(3) of line to where it's reachable.

Every line returned by readline() has to be freed.  The call to free()
was unreachable because all paths heading towards it veered off,
resulting in a memory leak.  Move the free() to just after where the
line has been used for the last time.  Fixes 3a85e0bc9.

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.