Further tidy-up in DeleteItemGeneral.

One pair of cases in its switch (each pair is a test and its negation)
was sharing the code to compute the value, then flipping the answer
for one of the cases.  This involved retesting type after switching on
it; a little thought revealed how to avoid that; and the result can
readily be applied to every pair of cases.  While we're at it, match
may as well be a bool and a few blank lines can make it clearer what's
happening.
Add ItemIsInList() for use in an assertion.

Checks for an Item being in a list (in contrast to IsItemIn, which
checks for the list having an entry whose ->name strcmp()s the same as
a given string); this is needed in ConcatLists() to check we're not
creating a cycle.  It could also be used in reverse and delete
functions to check for cycles there, but at quadratic cost, where
concat only incurs linear cost (which it already had anyway).
Fixes and clean-up in ItemList2CSV_bound().

It's meant to concatenate a bunch of strings, joining them with a
separator.  However, it actually copied each string in turn to the
start of the buffer, while writing separators, and appending a final
'\0', in all the right places, had the content but been between them.

It also checked whether the space remaining was more than two less
than the length of the string it was about to copy into that space;
its comments make clear that it meant to leave two extra spaces, but
it actually left two too few.  It doesn't actually need two anyway, so
would have truncated the last byte if the buffer were just exactly big
enough.  Check separately, when appending a separator, for whether
there's space for it.

Its documentation also wrongly claimed that the string stored in the
buffer would have length equal to the buffer size on truncation (the
actual strlen() shall be one less) when it meant that the *return* is
the buffer size - hence *bigger* than strlen(buf), since we really do
'\0'-terminate - when we truncate.

... and re-write to use a pointer to where to write next, while
keeping track of remaining space as we go, with the -1 for '\0' taken
into account at the start.  It's just simpler that way ...
Lots of petty simplification and clarification in item_lib.c

Testing strlen(x) == 0 is just an expensive way to check x[0]; and,
while there is a case for extra parentheses to make operator
precedence overt, it's just ugly to over-do it.  Initialization is
best combined with declaration, in most cases.  Group related
activities together (e.g. alloc/free as one group, setting ->next as
another) and trust free(NULL) to be a safe no-op as specified.  Use
non-trivial booleans to avoid duplicating code.  Simplify !
Simplify SplitString.

Not every loop wants to be a for; if you find yourself having to undo
the increment step in some paths through the loop body, it's a good
hint that it doesn't.  Restructured the loop body as a simple
if/else-if/else chain in which a separator at the start is handled
just as a later unescaped separator, instead of duplicating that code.
Simplify SplitStringAsItemList().

Instead of using snprintf() to insert a character in a string, set the
string up sensibly in the first place so that we can simply copy the
character into the right place in it.  Instead of memset()ing 4k
in hopes that a failing sscanf won't touch the destination buffer,
actually check sscanf's return.
Potential nightmare-fix in DeleteItem().

When searching a list for the entry whose ->next is the one we're
about to delete, we'd stop on the last item, which doesn't have the
given entry as ->next (it has NULL).  We would then assume that, as it
(as opposed to its ->next) is non-NULL, we should hang our item's
->next off it, where it never did belong.  If the item we're deleting
had our supposed list head somewhere down->next of it, this could have
created a cycle.  So let the loop variable fall off the end of the
loop as NULL and do at least assert that a non-NULL alleged
predecessor has the node to be deleted as its ->next, before fixing
that.
Purge code for unused backup file name.

While there may be some sense in backing up a file before over-writing
it, and computing the name for that file would be useful if we did, we
don't.  So don't bother cooking up an unused name.
Log() on error when reading an item list.
Document what ListsCompare really tests (approximating equality).
Eliminate ByteSizeList in favour of ItemListSize.

Both computed the sum of lengths of strings in a list.  The sole
difference is that the latter checks for, and skips, NULL entries, on
which the former would crash.  Code outside core uses the latter, only
code inside core uses the former, so this change is self-contained to
core (easier to git administer).

Also make ItemList2CSV() robust against NULL entries, now that it's
delegating size computation to ItemListSize().
contrib/reindent.pl: allow explicit mode spec on the command line
Merge pull request #1542 from jimis/minor

Do not truncate even the longest debug output
Record agent info log for all runs even for non debug runs.
body server control: allowlegacyconnects

Which IPs or subnets to allow connections from, even though they are not
using the latest protocol.
Stylistic changes.
Fix old protocol client_code segfault.

Also reverted opaqueness of ConnectionInfo and restored direct access to
it for the relevant client_code. I expect that at least for libcfnet
internal usage I don't have to deal with abstraction layers.
Merge pull request #1545 from jimis/minor

Record agent info log for all runs even for non debug runs.
Silence warning when compiling without XML.
Moving documentation in .c files from header files.

This is documentation the programmer has to see the moment he looks up
the function name, and currently most (all?) editors take you to the
implementation, not the declaration.