The old behaviour is still present under an ifdef based on _MSC_VER,
so it should still appear in the w32old builds we're still making.

(cherry picked from commit 49b91bc1)

(And remove another reference to connection type 'buttons'.)

No functional change, probably.

PuTTYgen and its documentation are pretty consistent about calling their
encryption key a 'passphrase', as opposed to a 'password' supplied
directly to a server; but the Argon2 parameters UI reverted to
'password hash', which seemed unecessarily confusing.

I think it's better to use the term 'passphrase' consistently in the UI.
(People who are used to Argon2 being called a 'password hash' can
probably deal.)

This required tweaking the coordinates of the Windows PuTTYgen UI.

Instead of 'Kb', which could be misread as 'Kbit'.

If we're publishing the server, then we should say something about the
fact that this option exists to talk to it. Also, if the option exists
on the front page at all in a released version of PuTTY, it behooves
us to document it slightly more usefully than just a handwave at 'this
is specialist and experimental'.

I've been using and testing it for a while now, in various
circumstances, and I think it's reasonably robust.

And if a release version of PuTTY is going to have an option on the
front page that basically exists to talk to psusan, it seems silly to
then turn round and say "But you can't have psusan itself, so nerrr".
So let's not.

Also, Jacob points out that the sooner we encourage distro maintainers
to package psusan, the sooner it will (eventually) percolate through
into all the places where it would be convenient for it to already
exist, like the insides of containers.

SUPDUP came, at my insistence, with a history section in the docs
for people who hadn't heard of it. It seems only fair that the
other obsolete network protocols (or, at least, the ones we *wish*
were obsolete :-) should have the same kind of treatment.

Moved the Raw protocol to below Serial, so that the first two
sections are SSH and Serial, matching the (now very emphatic)
priority order in the config UI.

Similarly, reordered the bullet points in \k{config-hostname}.

They're not any more!

Just noticed that intro.but still gives it equal weight with Unix,
which probably wasn't even true in 2001, and certainly isn't true now.

I've filled in some text about prime generation methods and Ed448,
which were all the things marked as 'review before release'.

While I'm at it, also filled in a reasonable enough DSA key length
recommendation, because the FIXME comment in that section was within
sight of one of the places I was editing. FIPS 186-4 seemed to think
that RSA and DSA had comparable relationships between the key length
and practical security level, so I see no reason not to use the same
recommendation for both key types.

In commit bb59f273 I changed a use of the constant GWL_ID to
GWLP_ID, on the grounds that the former caused a build failure under
winelib. But the GWLP constants are supposed to be used with
GetWindowLongPtr, and I was still calling GetWindowLong.

(Benign, since the two sets of constants are the same. But that is the
only case in the whole code base where I'd made that error, and since
it was only introduced a couple of days ago, there's no possibility of
a longstanding historical reason for carefully not touching it!)

I must have written that comment before I'd decided on how to spell
--setver.

'User options' is a technical term in SUPDUP, but I don't think
'Location string' is one of them.

Turns out that the precautions against winelib builds failing, which I
put in years ago because I was using winelib as a build setup for
Coverity testing, are all obsolete. My Coverity build scripts runs
fine now without any of them.

Aah, that's better. It's been bugging me since I added it.

Many of VS's warnings are too noisy to be useful, but I just tried the
experiment of turning off the unrecoverable ones and seeing what was
left, and I found a couple of things that actually seem worth fixing.

In a few cases in mpint.c, and in one case in sshzlib.c, we had the
idiom 'size_t var = 1 << bitpos;', and VS pointed out that when '1' is
implicitly a 32-bit int and 'size_t' is 64 bits, this is probably not
what you wanted. Writing '(size_t)1 << bitpos' is safer.

Secondly, VS complained about lots of functions failing to return a
value, or not returning a value on every code path. In every case this
was somewhere that we'd used the local unreachable() idiom to indicate
that those code paths didn't return at all. So the real problem was
that that idiom didn't work in VS. And that's not because VS _can't_
mark functions as noreturn: it has a perfectly good declspec for it.
It was just that we hadn't actually _done_ it. Now added a clause in
the #if in defs.h that spots VS and uses the declspec.

In commit d53b3bcd I changed the final setting of kl->broken
so that it wouldn't overwrite a 'true' value set earlier in the
function. But that means it might not be set at all, because I forgot
I now needed to initialise it to false. Ahem.

This replaces the pure radio-button setup that we've always had on the
Session config panel.

Since the last release, that set of radio buttons has been getting out
of hand. We've added two new protocols (SUPDUP, and the 'bare
ssh-connection' aka psusan protocol), neither of which is mainstream
enough to be a sensible thing to wave at all users on the front page
of the config GUI, so that they perhaps start wondering if that's the
protocol they want to use, or get sidetracked by going and looking it
up.

The replacement UI still has radio buttons, but only for the most
common protocols, which will typically be SSH and serial. Everything
else is relegated to a drop-down list sitting next to a third radio
button labelled "Other".

In every be_* module providing a backends[] list, there's also a
variable n_ui_backends which indicates how many of the backends ought
to appear as first-level radio buttons.

(Credit where due: this patch is a joint effort between Jacob and me,
and is one of those rare cases where it would be nice to be able to
put both our names into the Author field of the commit. Failing that,
I can at least mention it here.)

This will let us put two controls side by side (e.g. in disjoint
columns of a multi-col layout) and indicate that instead of the
default behaviour of aligning their top edges, their centreline (or,
even better if available, font baseline) should be aligned.

NFC: nothing uses this yet.

We never expect to be passed a NULL GtkFrontend pointer, and even if
we were, we'd have crashed several lines above this test.

It was benign, of course, but Coverity (which pointed it out) dislikes
this kind of thing on the basis that it's confusing - you ought to
either test it for NULL properly, or not at all - and I see its point.

Coverity points out that it's theoretically possible for the main loop
in radioline_common() to read r.bottom without having gone through the
conditional setup at the start of the function _or_ a previous
iteration of the main loop. I think this can only happen in some silly
case that doesn't actually come up, but on the other hand, it's easy
to add the necessary robustness.

Coverity pointed out that I'd checked if the LoadedFile was NULL, set
an error message ... and then accidentally fallen through to the
success handler anyway.

Coverity points out that if rsa_ssh1_public_blob_len sees data it
doesn't like, it returns -1 to indicate an error. But the code that
uses it to parse the SSH1_AGENT_RSA_IDENTITIES_ANSWER payload was
passing it directly to get_data() as a length field, without checking
for that case. Now we do check it, and use it to set the existing
kl->broken flag that indicates that the key list was not correctly
formatted.

Coverity was unhappy that I'd used the packet length as a loop bound
without sanitising it first (on the basis that it had decided anything
coming from GET_32BIT_MSB_FIRST was potentially tainted).

I think this is not a security issue: all that will happen if the
server sends a huge packet length is that we'll try to allocate space
for it. On a 64-bit machine we might even _succeed_; on 32-bit, we'll
fail, and snewn() will abort the program rather than return NULL. So
*technically* this is a remote-triggered crash. But it can only happen
in a situation where the same server could have triggered the
termination of the SFTP connection just as easily by simply closing it
- the only difference is that the client would die with a different
fatal error message.

(In particular, it isn't even a DoS against other processes
participating in a connection-shared SSH session. The upstream will
pass the SFTP data stream through without parsing it, so it and the
other downstreams will be unaffected. Only the particular downstream
operating the SFTP client will run into this problem.)

If named_pipe_agent_gotdata was called with an error or EOF status, it
would call agent_cancel_query(pq), but then accidentally fall through
to the non-error handler which would dereference pq. I meant to return
early in that situation, and Coverity spotted that I'd left out the
early return statement.

Coverity objected to several similar cases in this code in which I'd
checked a pointer for NULL after already having done things to it. I
think all the cases are benign, in that (as the comments tersely
mention) those checks could only fail if the unifontsel system had got
_really_ confused, in which case probably some other bug would have
been on the point of manifesting anyway. But Coverity has a point
anyway: if I'm _going_ to check those values for NULL, let's check
them consistently.

Commit d851df48 deleted a #if / #else / #endif on the grounds
that the condition would now always be true, without also deleting the
code inside the #else. Happily, the then-branch ended with a return,
so it was a benign mistake - the erroneously left-in else-clause code
was unreachable. But now Coverity has pointed it out, let's remove it.

Coverity points out that we don't need to check the output buffer
bound before writing out the first 32 bytes of each full-length
BLAKE2b invocation, because the only time we're doing a full-length
one in the first place is if the output buffer bound was at least 64
bytes.

(More specifically: whenever we're in the while loop, length > 64, so
setting chunk = 32 and then checking if chunk > length has a totally
predictable answer.)