lib: Added [io]_stream_create_fd_*autoclose()
These make it clearer that the fd parameter will be closed automatically
when the stream is closed.

Eventually (in v2.3) we'll want to get rid of the autoclose boolean
parameter in [io]_stream_create_fd().

doveadm stats top: Fixed double-close()ing stats connection

director-test: Fixed double-close() on admin connection deinit

Added various asserts to try to silence Coverity false positives.

lib-mail: message_parser_parse_next_block() now fully clears block_r to be safe case.
I'm not away of this hitting any bugs, but better to be safe.

Added "fall through" comments to switch statements where break is intentionally missing.
This should make Coverity quiet about these..

lib-storage: Fixed parsing corrupted mailbox list index header.
Duplicate IDs should have caused an error instead of being silently ignored.
Found by Coverity

imap-urlauth: Fatal failure error handling wasn't done correctly.
Found by Coverity

replication plugin: Synchronous notification timeout error wasn't logged as intended.
Found by Coverity

lib-otp: OTP_MAX_WORD_LEN wasn't actually enforced, any word lengths could have been used.
Doesn't look like this could have caused any real problems.
Found by Coverity

fts: Improved doveadm fts dump for corrupted expunge log
Although we may still be trying to allocate up to 2 GB of memory, but at
least no more than that now.
Found by Coverity

lib: Fixed file_dotlock_replace(flags=DOTLOCK_REPLACE_FLAG_VERIFY_OWNER|DOTLOCK_REPLACE_FLAG_DONT_CLOSE_FD)
The verification check failed because fd was already set to -1 by that time.
Found by Coverity

director: Fixed crash if master_user_separator is set, but we didn't do a proxy lookup.
Found by Coverity

imapc: Avoid crashing if server happens to send invalid resp-text-codes.
If [KEY VALUE] is missing the VALUE, just set it to "" instead of NULL.
Found by Coverity

fts: Minor code cleanup: Don't increment NULL pointer.

Removed pointless NULL checks.
Found by Coverity

auth: Invalid userdb passwd-file and userdb templates may have caused crashes.
Using just "key" parameter instead of "key=value" usually worked, but for
some keys the code assumed that there was a value and it dereferenced NULL.
We'll solve this by just using value="" instead of value=NULL.
Found by Coverity

auth: passdb/userdb dict settings file parsing didn't handle errors correctly.
Found by Coverity

lib: Changed net_geterror() to return errno instead of -1 if getsockopt() fails.
None of the callers were actually checking for the -1 error value but
instead just passing it to strerror(). Since this error should just about
never happen it's better to just return a usable return value than try to
remember to handle errors that can't normally even happen.
Found by Coverity

doveadm sis deduplicate: Error handling fix if open() fails.
Found by Coverity.

lib: Added (void) prefixes to some setsockopt() calls which we don't care if they fail.

dsync: Fixed potential crash when debug logging was enabled.
Found by Coverity.

mbox: X-Delivery-ID: and X-IMAP: headers weren't dropped from incoming mails.
Because of a missing comma.. Found by Coverity.

doveadm who: Don't crash if server happens to send broken input.
Found by Coverity.

configure: Don't actually run the test to see if inotify works.
It's definitely no longer needed in modern Linux systems and the test itself
can also unintentionally fail sometimes.

lib: Changed hash_table_remove() "key not found" panic to be in a macro itself.
This makes it much easier to find out where such crashes are coming from.
Since this breaks the ABI in such a many places the ABI version number was
increased immediately..

lib: test-bits - fix nearest_power for 32-bit size_t
The test blindly went up to b=63, and the function correctly asserted.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

rawlog: Buffer writing to rawlog files to improve performance.

lib-fs: Fixed crash in fs-sis if hard linking failed.
(For example because there were too many hard links.)
Pointed out by Pavel Stano

lib: fix numpack overflow checking
As on broken input, bits may grow without limit, so << bits becomes
Undefined Behaviour. Add a simple check to the while loop to prevent
this.

Also, the (presumably) final byte adds something to the bit length,
so include that in the tally. If we didn't get to a final byte due
to the above while() condition, then this extra addition does no harm

Now we can precisely check for overflow conditions. Note that 64 bits
is perfectly OK, only 65+ is an overflow.

Note - no longer moving *p if there was a decode error.

Expand the test suite to check for overflow cases. Also checked for
short-input cases too, while I was there.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

lib: unit tests for lib/bits
Signed-off-by: Phil Carmody <phil@dovecot.fi>

lib-test: test_assert helper for loops
If you're repeatedly testing the same expression in a loop, it's
good to know where you are in the loop. Add an additional parameter
for these cases.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

lib: bit twiddles
bits_requiredXX() gives the number of bits required to store an unsigned
integer. Here, XX is 8, 16, 32, 64, reperesenting the size of the operand.
It belongs in the same file as nearest_power(), which makes most sense
in a separate bit twiddles file. Universal enough to stay in lib.h by
inclusion.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

lib: Fix MEM_ALIGN to cope with huge allocations
Attempting to allocate 2^32+1 bytes will look like it succeeds, as MEM_ALIGN
will set alloc_size = 8. The caller will then think it's got 4 gig to play
with.

e.g. t_malloc0 will wipe vast areas of memory before segfaulting, which might
include useful information we'd like in a corefile.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

lib-index: modseq -> {log file, offset} lookup often ignored dovecot.index.log.2
This caused the code to think that the modseq was too old and fall back into
slower sync.

So this change should fix the dsync "Modseq .. nom longer in transaction
log" warnings as well as improve IMAP QRESYNC efficiency.

lib: Added some kind of a unit test for hash table.
Just try out some insert+deletes randomly. Mainly I wrote this to check if
there is some obvious problem, but looks like not.

lib-ssl-iostream: Implement get_used_size() method.
Previously we were always returning that we didn't have anything buffered,
which could have caused huge memory usage (or malloc failures) with its
users (e.g. dsync).

login-common: Fixed potential crash at client disconnect.
Broken by recent change

login-common: Fixed infinite loop in ssl proxy flushing.

login proxy: Fixed connection hanging due to wrong ostream cork pairing

lib-storage: Mailbox list index updating didn't check properly if modseq tracking is enabled.
mail_index_modseq_enable() can be enabled (e.g. by virtual plugin) without
enabling MAILBOX_FEATURE_CONDSTORE.

(Actually the MAILBOX_FEATURE_* probably should be removed from lib-storage
entirely. They're too much of an IMAP feature.)

maildir: Mailbox list index refreshing shouldn't check cur/new dirs with maildir_very_dirty_syncs=yes
The recent mailbox_list_index_very_dirty_syncs change broke this. Just
because maildir_very_dirty_syncs=yes, it doesn't mean that
mailbox_list_index_very_dirty_syncs=yes also.

*-login: Flush SSL output when logging out.
The BYE and LOGOUT replies weren't being sent when they were sent from
imap-login process (before logging in).

doveadm fs delete: When doing recursive deletion, delete also the directories if needed.

dsync: Fixed renaming duplicate mailbox GUIDs.
We were trying to use only the last part of the hierarchical name instead of
the full name.

lib-ssl-iostream: Use SSL_MODE_RELEASE_BUFFERS if it exists to reduce memory usage.

mail-index: make uid_lookup_idx hold a sequence number, not a 0-based C array index
Rename it to _seq, and make it hold values 1.. rather than 0.. . Several
uses of the value are simplified by this change, and none are made more
confusing, so I think it helps improve maintainability of the code.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

mail-index: use _REC_AT_SEQ not _MAP_IDX in view_sync_get_log_lost_changes
Shifting i and j up by one to use in the new macro means we don't
need the +1's in the calls to view_sync_apply_lost_changes and
mail_index_map_lookup_keywords. Rename them as seq variables too.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

mail-index: use _REC_AT_SEQ not _MAP_IDX in backward loops
[seq-1 .. 0] becomes [seq .. 1]

Don't pre-decrement, and terminate before you process seq=0.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

mail-index: use _REC_AT_SEQ not _MAP_IDX in loops from [seq1..seq2]
[seq1-1..seq2) becomes [seq1..seq2]

Don't decrement before starting, and include the upper bound.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

mail-index: use _REC_AT_SEQ not _MAP_IDX in loops over every record
Just change the loop bounds from [0..count) to [1..count], and make the
loop variale explicitly a seq.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

mail-index: trivial robomatic migration from _MAP_IDX to new helper
All users which have a '-1' in their MAIL_INDEX_MAP_IDX have been changed
to the new mail_index_rec_at_seq() helper using this sed script:

$ sed -ie 's/MAIL_INDEX_MAP_IDX(\([^,]*[^)]*[^ ]\) \?- \?1)/MAIL_INDEX_REC_AT_SEQ(\1)/' src/lib-index/*.[ch]

No other users have been changed.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

mail-index: new helper macro with more user-friendly semantics
As the record ids range from 1..records_count, but the data is
stored as if in a C-style 0-based array, current clients of
MAIL_INDEX_MAP_IDX() must subtract 1 from the index themselved.

New MAIL_INDEX_REC_AT_SEQ() macro does the subtraction for you,
it gives you (the address of) the record from a seq number.

Uglified users of the former will be migrated to the latter.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

mail-index: optimise memmoves in expunge, only move each region once
Rather than shifting things back and back and back with potentially O(N^2)
(more precisely O(count*rec_count')) work factor, move each slice of memory
only once, directly where we want it to end up (O(rec_count') work factor).

Based on draft patch by Timo Sirainen.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

mail-index: trivial paranoia check in expunge loop
We will later be moving the zone between the current seqs and the
(end of the) previous seqs - this check ensures that the range is
indeed monotonic, and thus that zone is positive in size. It can
be zero right at the start, if the first seqs is (1,...)

Based on draft patch by Timo Sirainen.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

mail-index: Preparatory HACK - reverse the loop order in the expunge
This is the final step before the actual optimisation of the memmoves.

HACK, as it it de-optimises the moves so as much as possible gets moved
as many times as possible. It clears the path for a later patch which
optimises them far better.

Based on draft patch by Timo Sirainen.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

mail-index: hoist initialisation of mail index map out of expunge loop
Based on draft patch by Timo Sirainen.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

mail-index: explicitly inline contents of sync_expunge()
To later optimise the memmoves, we will need to be aware of previous seqs.
It's easier to just have the guts inlined, so too much state doesn't need
to be passed around.

For review, this change is best viewed with a whitespace-insensitive diff,
to verify that the new contents of the loop are identical to the contents of
the now-absorbed function.

Based on draft patch by Timo Sirainen.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

mail-index: call all expunge handlers first
Firstly, the init checking only needs to be done once. More importantly,
moving this preparatory stage into its own per-seqs loop means there's
less to juggle when we optimise the memmoving loop in subequent patches.

Based on draft patch by Timo Sirainen.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

mail-index: move expunge handler init into helper function
Preparatory for later optimisation patches where the call to this will
be lifted out of the expunge(seq1,seq2) inner function and done once
for the whole range of seqs in one go.

Based on draft patch by Timo Sirainen.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

director: If we detect that a user is being proxied to multiple backends, disconnect wrong connections.
Especially IMAP connections can otherwise stay alive for a long time and
cause problems.

doveadm director kick command added.
The kick gets sent to all the proxies within the director ring.

lib-index: Fixed somewhat random assert-crashes during extension resizes.

lib-index: Optimize removing large number of expunges.

lib-storage: "Message has no NUL characters" flag was being set wrong to cache file.
Only the first MIME part was checked for its existence.

When this flag was wrong, IMAP FETCH may have returned NUL characters
instead of converting them to 0x80 character. This apparently caused Outlook
to hang.

Compiler warning fix

auth: checkpassword callback callback type bike-shedding
This change doesn't change the compiler's (gcc) view on the correctness of
the code. It moves sparse's attention of where the potential issues are
though. Sparse used to complain about dodgy function pointer conversions on
both the way out (passing the callback function pointer), and on the way in
(entering the callback). Making the callback not lie about what it receives
gets rid of the way in warnings, but adds warnings as we pass the new
function pointer out. However, it already complains about that call anyway.
So it complains about 6 things in 3 functions rather than 6 things in 6
functions.

Of dubious worth, but it at least reduces the number of lines you need to
inspect to verify correctness.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

imap: exit imap_fetch_binary_init() identically on all failures
This changes the behaviour, as -1 is TRUE as a boolean.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

imap: use human-readable helper macro in remote_ip_is_usable
More readable, but helpfully shuts up sparse which complained about some
constants being long.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

imap: cmd_getmetadata_stream_continue returns bool, not int
The behaviour is unchanged, but we shouldn't pretend that -1 is different
from TRUE in a boolean context. Its only caller only cares about whether
it's 0 or not.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

indexer: fix indexer_queue_cancel_all behaviour
-1 is TRUE. Presumably -1 was intended to be passed to the callbacks via
indexer_queue_request_status_int(), not 100.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

auth: master-connection - bail on malformed list
If master is not communicating to us in a syntax we understand, just ask for
it to be unplugged. This changes the behaviour in this error case.

Previously, we returned -1, which is TRUE when converted to a boolean, and
thus this changes the error semantics, and may be horribly wrong. However,
the i_error()s in auth_master_input_line follow the same pattern.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

auth: set_credentials callback being passed an enum, not a bool
This changes the behaviour, as the error case is now mapped onto FALSE.
All non-zero values of course get squashed into true. Found by sparse.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

lib-otp: cast to the correct type of function pointer
sparse complains about the result of the F() cast being the wrong type
for the initialisation, which is true. So just cast to the right type
in the first place.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

imap: fix missing-command check
It's impossible for the command's pointer to be NULL at this point.
Previously, the command_find() would have returned NULL, but this check
presumably short-circuits that search in the trivial case, so has some
real use.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

lib-imap: API change - add const to imap_url *base parameter
We do not change what's there, therefore we can promise to not change what
is there.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

auth: sparse static cleanup, and some const cleanup
All the consts that are added to pointers represent deep const semantics.
There are other shallow consts that I've not added, as sometimes it's
better to not be const than have something you rely on change when you
think it won't.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

various - 'static' sparse cleanup
Signed-off-by: Phil Carmody <phil@dovecot.fi>

lib-mail: tests - trivial sparse cleanups
One static const, and one more obviously not-an-int integer.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

lib-imap: tests - trivial static and const sparse cleanups
Signed-off-by: Phil Carmody <phil@dovecot.fi>

lib-http: trivial sparse cleanups - statics and explicit NULLs
Signed-off-by: Phil Carmody <phil@dovecot.fi>

various - trivial NULL-related sparse cleanups
These zero-alikes are all pointers, so should explicitly be NULL.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

treewide - mass cleanup of 0 used to end a settings list
This also includes a change to the perl script which generates the
all-settings.c file.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

lib: pair VA_COPY with va_end
A va_copy creates a initialised va_list, as if a va_start had been done on it.
Therefore, pedantically, a va_end should also be done on it. On most platforms
this is a no-op, and for those where it isn't, the pairing is important.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

fts: parser-html - parser can fail on attributes='with values in single quotes'
If that value were to contain an odd number of double quotes, then the
HTML_STATE_TAG_(D)QUOTED state would be entered and not exited.

The two quoting types behave basically the same, so just add two new cases
and duplicate the state transition code.

fts: parser-html - parse_tag_name returns wrong value for comments
This function returns 1 more than the number of additional
characters to be swallowed up by the state transition.

fts: parser-html - parse_tag_name really does return an integer not a bool
It seems to return 1 more than the number of additional characters
(after '<') are swallowed up by the state change, not a bool. This
would imply that '3' is wrong.

lib-storage: functions returning bool actually return ints
Their return values are compared as if they are at least tri-state
(-ve, 0, +ve), so really aren't bools at all.

Note - this should cause the code to change in behaviour, and thus might
introduce regressions as previously all non-zeroes would have been mapped
to 1.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

lib-storage: context's dotlock_last_stale really is bool
It's only ever read as if it's a bool, so it really is a bool. Fix a write
treating it as an int.

Signed-off-by: Phil Carmody <phil@dovecot.fi>

lib-http: Fixed assertion failure in http_client_request_send_payload() caused by inappropriate retry attempt.

virtual: Crashfix: Don't deinitialize search args that haven't been initialized.

lib-storage: Avoid refreshing mailbox list index too often.

lib-storage: Optimize mailbox_exists() with mailbox_list_index=yes
We can return the existence from the index itself to avoid stat()s.

lib-storage: Added mailbox_list_index_very_dirty_syncs setting.
This setting assumes that the mailbox list index is up to date and uses it
without stat()ing backend mailbox files/dirs. (As a possible future TODO it
might be useful to still do the stat()ing, but only rarely.)

imap: If we're waiting for client to read data, show in process title how many bytes are buffered.

lib: Added o_stream_is_corked().

lib-imap: Use case-insensitive comparisons everywhere for imap_id_send setting parsing.
"os" and "os-version" were case-sensitive while others were not.
Patch by Apple.

virtual: If backend mailbox hasn't changed, we still need to build its uidmap.

imap: Fixed enabling METADATA if imap_capability string was explicitly set

lib-fs: Make sure we don't leak data stack.