Without this, the Conf objects in a session and its duplicate were
aliases of each other, which could lead to confusing semantic effects
if one of the sessions was reconfigured in mid-run, and worse still, a
crash if one session got cleaned up and called conf_free on a Conf
that the other was still using.
None of that was intentional; it was just a matter of forgetting to
clone the Conf for the duplicated session. Now we do.
Detecting that the child process in a pterm has terminated is
important for _any_ kind of pterm, so it's a mistake to put the signal
handler setup _solely_ inside the optional pty_pre_init function which
does the privileged setup and forks off a utmp watchdog process. Now
the signal handler is installed even in the GtkApplication-based
multi-window front end to pterm, meaning it will exist even on OS X.
ignore_sbar is a flag that we set while manually changing the
scrollbar settings, so that when those half-finished changes trigger
GTK event callbacks, we know to ignore them, and wait until we've
finished setting everything up before actually updating the window.
But somehow I had managed to leave the functions that actually _have
the effect_ (at least in GTK1) outside the pair of statements that set
and unset the ignore flag.
The effect was that compiling pterm for GTK1, starting it up, and
issuing a command like 'ls -l' that scrolls off the bottom of the
window would lead to the _top_ half of the ls output being visible,
and the scrollbar at the top of the scrollback rather than the bottom.
Apparently I haven't tested this compile mode in a while: I had a
couple of compile errors due to new code not properly #ifdeffed (the
true-colour mode has to be effectively disabled in the palette-based
GTK1 graphics model) and one for an unused static function
(get_monitor_geometry is only used in GTK2 and above, and with -Werror
that means I mustn't even _define_ it in GTK1).
With these changes, I still didn't get a clean compile unless I also
configured CFLAGS=-std=gnu89, due to the GTK1 headers having an
outdated set of ifdefs to figure out the compiler's semantics of
'inline'. (They seem to expect old-style gcc, which inconveniently
treats 'inline' and 'extern inline' more or less the opposite way
round from the version standardised by C99.)
My custom GTK layout class 'Columns' includes a linked list of
dynamically allocated data, and apparently I forgot to write a
destructor that frees it all when the class is deallocated, and have
never noticed until now.
While debugging some new code, I ran valgrind in leak-checking mode
and it pointed out a handful of existing memory leaks, which got in the
way of spotting any _new_ leaks I might be introducing :-)
This was one: in the case where an asynchronous agent query on Unix is
aborted, the dynamically allocated buffer holding the response was not
freed.
ATTR_REVERSE was being handled in the front ends, and was causing the
foreground and background colours to be switched. (I'm not completely
sure why I made that design decision; it might be purely historical,
but then again, it might also be because reverse video is one effect
on the fg and bg colours that must still be performed even in unusual
frontend-specific situations like display-driven monochrome mode.)
This affected both explicit reverse video enabled using SGR 7, and
also the transient reverse video arising from mouse selection. Thanks
to Markus Gans for reporting the bug in the latter, which when I
investigated it turned out to affect the former as well.
I've done this on a 'where possible' basis: in Windows paletted mode
(in case anyone is still using an old enough graphics card to need
that!) I simply haven't bothered, and will completely ignore the dim
flag.
Markus Gans points out that some applications which (not at all
unreasonably) don't trust $TERM to tell them the full capabilities of
their terminal will use the sequence "OSC 4 ; nn ; ? BEL" to ask for
the colour-palette value in position nn, and they may not particularly
care _what_ the results are but they will use them to decide whether
the right number of colour palette entries even exist.
Otherwise, moving the cursor (at least in active, filled-cell mode) on
to a true-coloured character cell causes it to vanish completely
because the cell's colours override the thing that differentiates the
cursor.
I'm not sure if any X11 monochrome visuals or Windows paletted display
modes are still around, but just in case they are, we shouldn't
attempt true colour on either kind of display.
This is a heavily rewritten version of a patch originally by Lorenz
Diener; it was tidied up somewhat by Christian Brabandt, and then
tidied up more by me. The basic idea is to add to the termchar
structure a pair of small structs encoding 24-bit RGB values, each
with a flag indicating whether it's turned on; if it is, it overrides
any other specification of fg or bg colour for that character cell.
I've added a test line to colours.txt containing a few example colours
from /usr/share/X11/rgb.txt. In fact it makes quite a good demo to run
the whole of rgb.txt through this treatment, with a command such as
perl -pe 's!^\s*(\d+)\s+(\d+)\s+(\d+).*$!\e[38;2;$1;$2;$3m$&\e[m!' rgb.txt
[unix/osxlaunch.c:133] -> [unix/osxlaunch.c:134]: (warning) Either the condition '!qhead' is redundant or there is possible null pointer dereference: qhead.
Alamy Liu points out that asking for CONF_host will display the wrong
part of the configuration in the case where serial port setup fails.
The Windows front end's analogous message already got this right, but
I must have forgotten to change this one too when I introduced
conf_dest.
This seems to work around a GTK 3.22 display bug that Colin Watson and
I have both observed on Ubuntu (though I found that proxying the X
server, e.g. by SSH X forwarding or xtruss, inhibited the bug). The
effect of the bug was that the terminal window would appear completely
black and nothing would ever be displayed in it, though the terminal
session was still actually running and keystrokes would be sent to it.
But changing the call to cairo_set_source_surface() to some other
cairo_set_source_foo caused successful drawing of whatever other
source I selected; the problem seemed specific to the image surface.
Also, when I popped up the Ctrl-right-click menu over the terminal
window, the menu didn't disappear when dismissed, i.e. the drawing
area's redraw operation was not drawing in black, but failing to draw
_anything_.
That led me to hypothesise that the draw event handler for the
terminal drawing area might somehow be accidentally inventing 0 rather
than 255 for the implicit alpha channel when using our RGB-type image
surface as a source; so I tried setting the surface type to one with
an explicit alpha channel in the hope that there would no longer be a
need to make up any alpha value at all. And indeed, that seems to
solve the problem for me, so I might as well commit it.
However, I don't know the full details of what the previous problem
was, so this is only an empirical workaround. If it turns out I was
making some other mistake without which a RGB source surface would
have worked for me, then I should probably revert this and do whatever
other fix turns out to be a better plan.
Calling gtk_widget_realize to enforce the existence of an underlying
GdkWindow, followed by gdk_window_ensure_native to enforce an
underlying X window in turn, allows me to get hold of an X window id
on which I can call the Xlib function for setting WM_CLASS, still
before the window is mapped.
With this change, plus Colin's preceding patches, the whole code base
_actually_ compiles and links against GTK 3.22 without any deprecation
warnings. (My claim in commit 8ce237234 that it previously did appears
to have been completely wrong - my guess is that I'd forgotten to
'make clean' before testing against 3.22 and so some source files had
already been compiled against earlier GTK headers.)
GTK+ 3.22 deprecates gdk_screen_{width,height} on the grounds that the
"screen" here actually refers to a virtual screen that may span multiple
monitors, and applications should generally be considering the width and
height of individual monitors. It's not entirely clear to me how this
fits with X geometry specifications, but I've gone with trying to get
hold of the geometry of the monitor that the window in question is on.
gdk_window_set_background was already deprecated, but with GTK+ 3.22
even gdk_window_set_background_rgba is deprecated, so we need a better
approach. The best seems to be to go with the flow and inject a custom
CSS style for the appropriate widgets.
GTK+ 3.22 deprecates gtk_menu_popup in favour of various
gtk_menu_popup_at_* functions. gtk_menu_popup_at_pointer seems most
appropriate, but that requires being able to pass it a GdkEvent rather
than just some elements of it. In order to achieve that, I've
rearranged the scroll_event shim to construct a real GdkEventButton and
pass that down to button_internal.
Minimal version of gtk+ 2.24 required to compile PuTTY
after GTK3 prep commits. Provide more compatibility macroses
to allow build against gtk+ 2.20.
Signed-off-by: Leonid Lisovskiy <lly.dev@gmail.com>
Unix PSCP, PSFTP, Plink and PuTTYgen now just report their build
platform as '64-bit Unix' or '32-bit Unix', without mentioning
irrelevant details of what flavour of GTK the other tools in the suite
might have been built against.
(In particular, they now won't imply anything outright untrue if there
was no GTK present at build time at all!)
Jacob pointed out the other day that the call to logevent with NULL
frontend handle can't possibly work, and the comment next to it saying
that it can is an outright lie (probably thoughtlessly copied from
some part of the Windows front end, where it actually would be true).
Furthermore, even if that logevent call didn't dereference NULL and
segfault, the followup call to fatalbox() would be inappropriate,
since proxied connections need not be the primary network connection
of the whole process.
Rewritten as a call to plug_closing, which is the proper channel
through which to report errors on an individual socket or equivalent.
When called with -V to ask for our version, return 0 rather than 1.
This is the usual behaviour observed by ssh(1) and other Unix commands.
Also use exit() rather than cleanup_exit() in pscp.c and psftp.c ; at
this point we have nothing to cleanup!
It's obvious to the trained eye whether GTK PuTTY was compiled against
GTK2 or GTK3, but the untrained eye would probably appreciate a little
help, and even the trained eye probably can't tell GTK 3.18 from 3.19
at a glance :-)
If we try to interpret a string argument as the name of a key file,
sometimes we it's in circumstances where we _know_ it's a key file, so
we must print an error message and return failure if the file can't be
loaded. Other times it's not, and we just fall back to interpreting
the argument in some other way (e.g. as a pattern match against the
comment or fingerprint of a key already in the agent).
My code dealing with failure returns from the public-key loading
functions were mishandling the latter case, if they identified a file
as existing and looking more or less like some kind of key file but
then it turned out to have a format error; they would try to copy and
return a public key that they didn't actually have. Even if
pageant_pubkey_copy avoided crashing as a result, this would still
inhibit the fallback to treating the input string as some other kind
of pattern match.
I think these were not strictly necessary, since passing a null
pointer to access(2) would have resulted in EINVAL rather than a
segfault. But it's clearer to put them in (and keeps static checkers a
bit happier).
Thanks, Coverity - I must have been lucky that Unix Pageant in client
mode hasn't so far happened to have this field come out non-NULL, or
else pageant_pubkey_copy would have tried to dupstr a garbage pointer.
Partly to reassure the user that they got what they asked for, and
partly so that's a clue for us in the logs when we get bug reports.
This involved repurposing platform_psftp_post_option_setup() (no longer
used since e22120fe) as platform_psftp_pre_conn_setup(), and moving it
to after logging is set up.
It's a function that exists on all platforms, not just on Unix - it's
used in ldisc.c - so it shouldn't have been declared only in unix.h.
Score another for clang's warnings.
Now, instead of returning a boolean indicating whether the query has
completed or is still pending, agent_query() returns NULL to indicate
that the query _has_ completed, and if it hasn't, it returns a pointer
to a context structure representing the pending query, so that the
latter can be used to cancel the query if (for example) you later
decide you need to free the thing its callback was using as a context.
This should fix a potential race-condition segfault if you overload an
agent forwarding channel and then close it abruptly. (Which nobody
will be doing for sensible purposes, of course! But I ran across this
while stress-testing other aspects of agent forwarding.)
backend_socket_log was generating the IP address in its error messages
by means of calling sk_getaddr(). But sk_getaddr only gets a SockAddr,
which may contain a whole list of candidate addresses; it doesn't also
get the information stored in the 'step' field of the Socket that was
actually trying to make the connection, which says _which_ of those
addresses we were in the middle of trying to connect to.
So now we construct a temporary SockAddr that points at the
appropriate one of the addresses, and use that for calls to plug_log
during connection setup.
If connect() returns EINPROGRESS, then previously we would detect a
successful connection by the socket becoming selectable for writing,
and spot an unsuccessful one by an error code being returned on the
first attempt to read from it.
This isn't the right way to do it: the right way is to respond to the
initial writability notification by calling getsockopt(SO_ERROR) to
retrieve the error code (if any) from the completed connection
attempt. Doing it the old way had the problem that when the socket
became writable, we could sometimes already have written some of our
outgoing data to it before finding out that the connect attempt failed
- which meant we'd discard that data from the bufchain, and no longer
have it to send through a later successful connection to a different
candidate address.
In case of connection errors before and during the handshake,
net_select_result is retrying with the next address of the server. It
however was immediately going to the last address as it was not
checking the return value of try_connect for all intermediate
addresses.
This shows the build platform (32- vs 64-bit in particular, and also
whether Unix GTK builds were compiled with or without the X11 pieces),
what compiler was used to build the binary, and any interesting build
options that might have been set on the make command line (especially,
but not limited to, the security-damaging ones like NO_SECURITY or
UNPROTECT). This will probably be useful all over the place, but in
particular it should allow the different Windows binaries to be told
apart!
Commits 21101c739 and 2eb952ca3 laid the groundwork for this, by
allowing the various About boxes to contain free text and also
ensuring they could be copied and pasted easily as part of a bug
report.
I had mistakenly pulled a 'char' value out of a string and passed it
to x11_font_has_glyph and x11_char_struct, each of which takes its two
index bytes as int-typed parameters. But if chars are signed, that
turns high-bit-set characters into out-of-range array indices. Oops.
The range checks in x11_char_struct prevented that from causing any
problem worse than refusal to display any affected glyph. Even so,
that's not particularly helpful. Fixed by changing the index byte
parameters to unsigned char type.
I noticed today that Unix Plink responds to SIGWINCH by accidentally
dying of EINTR having interrupted its main select loop, and when I
checked, there turn out to be a couple of other select loops with the
same bug.
The new font name configured by the keystrokes was missing its
"client:" or "server:" prefix, which could have led to the selection
of the wrong font in rare situations.
Each gtkfont back end now provides a routine that will return the name
of a similar font to the current one but one notch larger or smaller.
For Pango, this is just a matter of incrementing the font size field
in a standard way; for X11 server-side fonts, we have to go and do an
XListFonts query with a wildcard that requests fonts that vary only in
the size fields from the current one, and then iterate over the result
looking for the best one.
(I expect this will be more useful to Pango scalable-font users than
to X11 fonts, but it seemed a shame not to give the X11 side my best
shot while I was at it.)
Choice of hotkey: I know I'm being inconsistent with gnome-terminal's
use of Ctrl-plus and Ctrl-minus. I thought that was because I was
already using Ctrl-minus as a more convenient synonym for
Ctrl-underscore (which sends the actual control code 0x1F), but now I
actually try it, apparently I'm not. However, Ctrl-plus and Ctrl-minus
are quite horrible as a keystroke pair anyway (one has to be typed
with shift and one without!), and I feel as if the 'less' and
'greater' signs are more specific anyway, in that they specifically
indicate _size_ rather than just 'unspecified numerical value'.
There were already two places in the code (x11font_enum_fonts and
x11_guess_derived_font_name) where we retrieved an XLFD from the X
server, sawed it up ad-hoc into its '-'-separated parts and accessed
them by numeric index.
I'm about to add a third, so before I do, let's turn this into a
somewhat principled system where we get to do the decode/encode in
just one place and call all the individual fields by names that are
actually memorable.
No functional change intended by this commit.
The XDG configuration location ($XDG_CONFIG_HOME/putty, or
~/.config/putty) is now prefered over the old ~/.putty location, if the
XDG location already exists. If it doesn't exist, we try to use one of
the old locations ($HOME/.putty, [/etc/passwd home]/.putty, /.putty). If
none of the directories exist, we fall back to ~/.config/putty or
~/.putty, if the XDG_DEFAULT macro is defined or not, respectively. The
PUTTYDIR environment variable remains a definitive override of the
configuration location. This all ensures that the old location is still
used, unless the user explicitly requests otherwise.
The configuration directories are created using the make_dir_path()
function, to ensure that saving the configuration doesn't fail e.g.
because of a non-existent ~/.config directory.
Essentially 'mkdir -p' - we try to make each prefix of the pathname,
terminating on any error other than EEXIST. Semantics are similar to
make_dir_and_check_ours(): we return NULL on success or a dynamically
allocated error message string on failure.
This should avoid the possibility of the SIGWINCH handler's blocking
when trying to write to the pipe. This could only happen if we'd
somehow received PIPE_BUF SIGWINCHes without reading the pipe, which
would be difficult to achieve.
While we're at it, also set O_NONBLOCK on the reading side of the pipe,
just in case.
A side effect of commit 1f9df706b seems to have been to squash those
areas right up against the bottom of the dialog box, which is ugly. I
don't fully understand why it only happens to those drawing areas and
not to buttons placed in the fake 'action area' by other dialogs, but
anyway, adding an explicit margin-bottom attribute seems to solve it.
This is another widget that can appear in the top-level window, in
addition to the drawing area and scrollbar we put there ourselves, and
hence which needs to be accounted for when figuring out the
relationship between the drawing area size in character cells and the
full window size in pixels.
Finding the menu bar widget itself is a bit of a hassle, but having
found it, dealing with it is basically the same as dealing with the
scrollbar, only with x and y swapped.
This function, which parses the X11-style '-geometry WxH+X+Y' option
argument and automatically loads the result into the window, is also
being deprecated.
Fortunately we already had a fallback option for GTK1 (which didn't
have gtk_window_parse_geometry in the first place), calling the Xlib
geometry-parsing function and manually loading the results into GTK.
The method of loading into GTK is not the same between the two
versions, but the basic strategy is still viable.
For the sake of maintaining and testing fewer ifdef branches, I've
removed the use of gtk_window_parse_geometry _completely_, even in
GTK2 which did have it. GTK2 now uses the same strategy that I've
switched to for GTK3.
gtk_window_resize_to_geometry and gtk_window_set_default_geometry are
deprecated as of GTK 3.20, so now we do the geometry -> pixel size
conversion on our side.
This is preparation for dealing with the fact that GTK's geometry-
based API routines for setting the window size are being deprecated:
we'll no longer be able to specify a width/height in characters and
have GTK convert that into a pixel size based on the geometry hints
we'd already fed it. So we'll need to do that conversion ourselves,
and the easiest approach is to make it easy to recompute the geometry
hints on our side whenever we need them.
gdk_device_grab and all its preparatory faff are now deprecated, and
gdk_seat_grab is the new thing. Introduce yet another branch to all
the ifdefs for keyboard-grabbing. On the plus side, at least it's
slightly simpler than the GdkDevice business.
That function is deprecated as of 3.18, on the basis that GTK doesn't
need telling any more when the adjustment's owning widget needs
updating. So we just need to condition out the call.
Protecting our processes from outside interference need not be limited
to just PuTTY: there's no reason why the other SSH-speaking tools
shouldn't have the same treatment (PSFTP, PSCP, Plink), and PuTTYgen
and Pageant which handle private key material.
E.g. you might pass '--random-device=/dev/urandom'.
Mostly because I got sick of waiting for /dev/random to finish
blocking while I was trying to generate throwaway keys for testing bug
fixes in cmdgen itself. But it might also be useful on systems that
call their random device by a different name that we haven't
encountered.
(Since cmdgen also reads the saved PuTTY random seed file, setting
this option to /dev/zero will not render key generation deterministic.
It's tempting to provide _some_ way to do that, for testing purposes
and clearly marked as dangerous of course, but I think it would take
more faff than this.)
Partly this is because the geometry_widget functionality is going away
in a later version of GTK3, so sooner or later we'll need not to be
using it anyway. But also, it turns out that GTK 3's geometry
calculations have the unfortunate effect of setting the window's base
and min heights to values that are not congruent mod height_increment
(because the former is the value we gave, but the latter is based on
the minimum height of the scrollbar), which confuses at least one
window manager (xfwm4) and causes the window to be created one row too
small.
So I've redone all the geometry computations my own way, based on the
knowledge that the only widgets visible in the top-level window are
the drawing area and the scrollbar and I know how both of those
behave, and taking care to keep base_height and min_height congruent
to avoid that xfwm4 bug.
If you're connecting to a new server and it _only_ provides host key
types you've configured to be below the warning threshold, it's OK to
give the standard askalg() message. But if you've newly demoted a host
key type and now reconnect to some server for which that type was the
best key you had cached, the askalg() wording isn't really appropriate
(it's not that the key we've settled on is the first type _supported
by the server_, it's that it's the first type _cached by us_), and
also it's potentially helpful to list the better algorithms so that
the user can pick one to cross-certify.
It won't return true, because pterm's use of conf is a bit nonstandard
(it doesn't really bother about the protocol field, and has no use for
either host names _or_ serial port filenames). Was affecting both
gtkapp and gtkmain based builds.
The About box is where it showed up most obviously that I'd hastily
bunged a GtkBox inside another GtkBox without considering their
margins: the 'action area' had twice the margin it should have had and
the rightmost button didn't align with the right edge of the rest of
the window contents.
Easily fixed by giving the inner hbox margin 0 (fixing the right align
and the excessive space around all the buttons), and using the
'spacing' property of GtkBox to ensure multiple buttons in it are
nicely separated without having to take care over that in the client
code that adds them.
This commit adds two .plist files, which go in the app bundles; two
.bundle files, which are input to gtk-mac-bundler and explain to it
how to _create_ the bundles; and a piece of manual addition to
Makefile.am that actually runs gtk-mac-bundler after building the
gtkapp.c based binaries and the OSX launcher. The latter is
conditionalised on configuring --with-quartz (unlike the binaries
themselves, which you can build on other platforms too, though they
won't do much that's useful).
The big problem with making an OS X application out of a GTK program
is that it won't start unless DYLD_LIBRARY_PATH and several other
environment variables point at all the GTK machinery. So your app
bundle has to contain two programs: a launcher to set up that
environment, and then the real main program that the launcher execs
once it's done so.
But in our case, we also need pterm to start subprocesses _without_
all that stuff in the environment - so our launcher has to be more
complicated than the usual one, because it's also got to save every
detail of how the environment was when it started up. So this is the
launcher program I'm going to use. Comments in the header explain in
more detail how it'll work.
Also in this commit, I add the other end of the same machinery to
gtkapp.c and uxpty.c: the former catches an extra command-line
argument that the launcher used to indicate how it had munged the
environment, and stores it in a global variable where the latter can
pick it up after fork() and use to actually undo the munging.
When it's finished, this will be the backbone of the OS X GTK port:
using a GtkApplication automatically gives us a properly OS X
integrated menu bar.
Using this source file in place of gtkmain.c turns the usual Unix
single-session-per-process PuTTY or pterm into the multi-session-per-
process OS X style one.
Things like Duplicate Session can be done much more simply here - we
just grab the Conf * from the source window and launch a new window
using it, with no fiddly interprocess work needed.
This is still experimental and has a lot of holes, but it's usable
enough to test and improve.
This is a weird thing to have to do, but it is necessary: the OS X
PuTTY will need its top-level windows to be instances of a thing
called GtkApplicationWindow, rather than plain GtkWindow. Hence, the
actual creation of windows needs to be somewhere that isn't
centralised between the two kinds of front end.
Instead of main() living in uxputty.c and uxpterm.c, and doing a
little bit of setup before calling the larger pt_main() in gtkmain.c,
I've now turned things backwards: the big function in gtkmain.c *is*
main(), and the small pieces of preliminary setup in uxputty.c and
uxpterm.c are now a function called setup() which is called from
there. This will allow me to reuse the rest of ux{putty,pterm}.c, i.e.
the assorted top-level bits and pieces that distinguish PuTTY from
pterm, in the upcoming OS X application that will have its own main().
This lays further groundwork for the OS X GTK3 port, which is going to
have to deal with multiple sessions sharing the same process. gtkwin.c
was a bit too monolithic for this, since it included some
process-global runtime state (timers, toplevel callbacks), some
process startup stuff (gtk_init, gtk_main, argv processing) and some
per-session-window stuff.
The per-session stuff remains in gtkwin.c, with the top-level function
now being new_session_window() taking a Conf. The new gtkmain.c
contains the outer skeleton of pt_main(), handling argv processing and
one-off startup stuff like setlocale; and the new gtkcomm.c contains
the pieces of PuTTY infrastructure like timers and uxsel that are
shared between multiple sessions rather than reinstantiated per
session, which have been rewritten to use global variables rather than
fields in 'inst' (since it's now clear to me that they'll have to
apply to all the insts in existence at once).
There are still some lurking assumptions of one-session-per-process,
e.g. the use of gtk_main_quit when a session finishes, and the fact
that the config box insists on running as a separate invocation of
gtk_main so that one session's preliminary config box can't coexist
with another session already active. But this should make it possible
to at least write an OS X app good enough to start testing with, even
if it doesn't get everything quite right yet.
This change is almost entirely rearranging existing code, so it
shouldn't be seriously destabilising. But two noticeable actual
changes have happened, both pleasantly simplifying:
Firstly, the global-variables rewrite of gtkcomm.c has allowed the
post_main edifice to become a great deal simpler. Most of its
complexity was about remembering what 'inst' it had to call back to,
and in fact the right answer is that it shouldn't be calling back to
one at all. So now the post_main() called by gtkdlg.c has become the
same function as the old inst_post_main() that actually did the work,
instead of the two having to be connected by a piece of ugly plumbing.
Secondly, a piece of code that's vanished completely in this
refactoring is the temporary blocking of SIGCHLD around most of the
session setup code. This turns out to have been introduced in 2002,
_before_ I switched to using the intra-process signal pipe strategy
for SIGCHLD handling in 2003. So I now expect that we should be robust
in any case against receiving SIGCHLD at an inconvenient moment, and
hence there's no need to block it.
When we're displaying bidirectionally active text (that is, text that
the Unicode bidi algorithm will fiddle with), we need to suppress
Pango's bidi because we've already done our own. We were doing this by
calling is_rtl() on each character, and if it returned true,
displaying just that character in a separate Pango call.
Except that, ahem, we were only doing this if the _first_ character
encountered during a scan of the display buffer was rtl-sensitive. If
the first one was fine but a subsequent one was rtl-sensitive, then
that one would just get shoved into the buffer we'd already started.
Running pterm -fn 'client:Monospace 12' and displaying
testdata/utf8.txt now works again.
When I cut it in half so I could fetch the XCharStruct for a given
character, I forgot that the remaining half should check whether it
had got NULL from the XCharStruct finder. Ahem.
If you run something like 'seq 2000000000' in a GTK3 pterm, the window
never actually updates, because pterm always considers reading more
data from the pty to have higher priority than delivering the "draw"
event. Using g_io_add_watch_full instead of g_io_add_watch allows us
to explicitly lower the priority of the I/O sources, so that window
redraws will take precedence.
This is quite a pain, since it involves inventing an entire new piece
of infrastructure to install a custom Xlib error handler and give it a
queue of things to do. But it fixes a bug in which Unix pterm/PuTTY
crash out at startup if one of the root window's CUT_BUFFERn
properties contains something of a type other than STRING - in
particular, UTF8_STRING is not unheard-of.
For example, run
xprop -root -format CUT_BUFFER3 8u -set CUT_BUFFER3 "thingy"
and then pterm without this fix would have crashed.
I had completely forgotten, when rendering each glyph to a server-side
pixmap and downloading its contents, to only look at the part of the
pixmap that XDrawImageString would have overwritten, as specified by
the metrics in the XCharStruct. Now 'pterm -fn server:variable'
doesn't randomly make up bitmap nonsense outside each character's
bounding rectangle.
GCC 6 emits strict-aliasing warnings here, so use the existing
sockaddr_union arrangements to avoid those. As a prerequisite for being
able to express sk_tcp_peer_info in terms of sockaddr_union, I fixed up
the union elements to be a bit less odd in the NO_IPV6 case.
Now all the uses of the licence text or the short copyright notice get
it from a new header "licence.h", which in turn is built by a Perl
script licence.pl invoked by mkfiles.pl, using LICENCE itself as the
source.
Hence, I can completely remove a whole section from the list of
licence locations in CHECKLST.txt :-)
I've made the licence text, the About box, and the host key dialog
into GTK selectable edit controls. (The former because it contains a
lot of text; the About box because pasting version numbers into bug
reports is obviously useful; the host key because of the fingerprint.)
When we provide an editable text box with a drop-down list of useful
preset values, such as the one full of character sets in the
Translation panel, we implement it on GTK 2.4+ as a GtkComboBox
pointing at a two-column GtkListStore, in which the second column is
the actual text (the first being a numeric id). Therefore, we need to
set the "entry-text-column" property to tell GtkComboBox which of
those columns to look in for the value corresponding to the edit-box
text.
Thanks to Robert de Bath for spotting the problem and tracing it as
far as commit 5fa22495c. That commit replaced a widget construction
call via gtk_combo_box_entry_new_with_model() with one using the newer
gtk_combo_box_new_with_model_and_entry(), overlooking the fact that
the former provided the text column number as a parameter, and the
latter didn't.
I'd missed out an if statement in the Unix proxy stderr code
introduced by commit 297efff30, causing ret->cmd_err to be passed to
uxsel_set even when it was -1 (which happened in the non-GUI tools).
Unfortunately, putting a negative fd into the uxsel tree has really
bad effects, because the first_fd / next_fd interface returns a
negative number to signal end-of-list - and since the uxsel tree is
sorted by fd, that happens _immediately_.
Added the missing if statement, and also an assertion to make sure we
never pass -1 to uxsel_set by mistake again!
gtk_misc_set_alignment was deprecated in GTK 3.14. But my replacement
code using gtk_label_set_xalign doesn't work there, because that
function wasn't introduced until GTK 3.16, so there are two minor
versions in the middle where a third strategy is needed.
It has three settings: on, off, and 'only until session starts'. The
idea of the last one is that if you use something like 'ssh -v' as
your proxy command, you probably wanted to see the initial SSH
connection-setup messages while you were waiting to see if the
connection would be set up successfully at all, but probably _didn't_
want a slew of diagnostics from rekeys disrupting your terminal in
mid-emacs once the session had got properly under way.
Default is off, to avoid startling people used to the old behaviour. I
wonder if I should have set it more aggressively, though.
On both Unix and Windows, we now redirect the local proxy command's
standard error into a third pipe; data received from that pipe is
broken up at newlines and logged in the Event Log. So if the proxy
command emits any error messages in the course of failing to connect
to something, you now have a fighting chance of finding out what went
wrong.
This feature is disabled in command-line tools like PSFTP and Plink,
on the basis that in that situation it seems more likely that the user
would expect standard-error output to go to the ordinary standard
error in the ordinary way. Only GUI PuTTY catches it and logs it like
this, because it either doesn't have a standard error at all (on
Windows) or is likely to be pointing it at some completely unhelpful
session log file (under X).
I've defined a new value for the 'int type' parameter passed to
plug_log(), which proxy sockets will use to pass their backend
information on how the setup of their proxied connections are going.
I've implemented support for the new type code in all _nontrivial_
plug log functions (which, conveniently, are precisely the ones I just
refactored into backend_socket_log); the ones which just throw all
their log data away anyway will do that to the new code as well.
We use the new type code to log the DNS lookup and connection setup
for connecting to a networked proxy, and also to log the exact command
string sent down Telnet proxy connections (so the user can easily
debug mistakes in the configured format string) and the exact command
executed when spawning a local proxy process. (The latter was already
supported on Windows by a bodgy logging call taking advantage of
Windows in particular having no front end pointer; I've converted that
into a sensible use of the new plug_log facility, and done the same
thing on Unix.)
We set up a pair of bufchains for the standard input and output
exchanged with the proxy process, but forgot to clear them when the
Local_Proxy_Socket is cleaned up.
TOOLTYPE_NONNETWORK (i.e. pterm) already has "-log" (as does Unix
PuTTY), so there's no sense suppressing the synonym "-sessionlog".
Undocumented lacunae that remain:
plink accepts -sessionlog, but does nothing with it. Arguably it should.
puttytel accepts -sshlog/-sshrawlog (and happily logs e.g. Telnet
negotiation, as does PuTTY proper).
This saves the need to fork and exec "cat", which should speed things
up. It also ensures that the network output goes to /dev/null, which
should avoid problems with blocking when writing to a full pipe.
In GTK3, the line 'Continue with connection?' got wrapped (in spite of
my attempt to enforce via string_width() that it didn't - probably a
few pixels were needed on top of that for various padding and
furniture) so it looked even sillier. But it looked a bit narrow to be
sensible even in GTK2, so the simplest answer is just to widen it
considerably.
I think it only did so in GTK2 by virtue of the About box being a
GtkDialog. But in GTK3 I've abandoned GtkDialog for not being flexible
enough, so I have to process the Escape key myself.
The askalg() dialog, and several one-button things like the licence
box, have no button labelled 'cancel'. But in all cases we do want
Escape to terminate the box, with as negative an answer as is
available. So now we assign the 'iscancel' flag to any button whose
numeric value is the smallest of the ones given as input to
messagebox().
(In a one-button box, this leads to isdefault and iscancel _both_
being set for that button. That's fine; it works.)
I had put an entire piece of code into win_key_press's SHORTCUT_UCTRL
handler to carefully handle all the different kinds of list box
control and do something sensible with each one, and then I just went
and used a generic SHORTCUT_FOCUS type shortcut instead of actually
_calling_ all that carefully prepared code.
Now selecting (say) the character-classes list box in the Selection
panel using its Alt-e shortcut works; also, shortcut-selecting a popup
menu such as the ones in the Bugs panel causes the menu to pop up,
which I think is nicer than what previously happened.
I'd failed to set the widget field in its shortcut structure, leading
to an annoying GTK warning log message and no useful UI action when
Alt-G was pressed in the config box.
I had originally planned to implement a Compose-type key locally in
GTK PuTTY, as I did in Windows PuTTY. But in fact we've done this for
some time by delegating to the GTK IM system, which is a far better
idea anyway. So there's no point any more having the FIXME comment
that mentions Compose keys.
Also, there was a comment worrying about what I was going to do about
double-width characters in Pango, which is long since sorted out.
Fixes a behaviour which I intended all along but apparently didn't
work before on GTK: if you start PuTTY, _select_ a saved session in
the list box but don't hit Load, and then just hit Open, then it will
be implicitly loaded and run for you, as a special case to save you an
extra button-press.
This depends on noticing that the saved-sessions list box last had the
focus, for which I need my widget_focus() handler to be called for
basically all config widgets so that I can track what _did_ last have
focus. Unfortunately, I missed a couple out.
The previous sequence of events was that I would display the window
synchronously (via gtk_widget_show_now), so that I knew it was
actually on the screen and realised as an X window, and then I'd grab
the keyboard, and once the keyboard was grabbed, connect up the
keyboard event handlers and display the prompt.
I have to assume that deferring the display of the 'enter the
passphrase' prompt until the keyboard handlers were set up was
intended as some sort of 'not misleading the user' measure - don't
tell them to start typing until we're actually ready to start typing.
But unfortunately it has the side effect that the window is displayed
at a much smaller size before the prompt label appears, and centred on
the screen according to _that_ size - and then we display the prompt
label and the window resizes and is now off-centre. So I think it's
better not to try to be clever, and just make the window come up at
the right size initially.
(Actually, it looks as if nothing in the window is actually drawn
until that whole init function is finished anyway, so the prompt label
_already_ doesn't get physically displayed too early. So the whole
idea was pointless in the first place!)
When displaying a server-side font, the unified font selector's
font-style list box contains some lines which are character-set
headings, and others which are actually selectable font styles. We tag
the former with the "sensitive"=FALSE attribute, to prevent them from
responding to clicks. In GTK2, this also made them visually distinct
from the normal lines, by greying them out; in GTK3 it makes no visual
difference.
The simplest solution is to bold those lines, hinting that they're
sort of section headings. That looks OK in GTK2 as well, so I've done
it unconditionally.
The top-level loop in gtkwin.c which draws text was expecting that the
right way to draw a printing character plus combining characters was
to overprint them one by one on top of each other. This is an OK
assumption for X bitmap fonts, but in Pango, it works very badly -
most obviously because asking Pango to display a combining char on its
own causes it to print a dotted circle representing the base char, but
also because surely there will be character combinations where Pango
wants to do something more sophisticated than just printing them each
at a standard offset, and it would be a shame not to let it.
So I've moved the previous overprinting loop down into the x11font
subclass of the unifont mechanism. The top-level gtkwin.c drawing code
now calls a new method unifont_draw_combining, which in the X11 case
does the same loop as before, but in the Pango case, just passes a
whole base+combinings string to Pango in one go and lets it do the
best job it can.
Touchpad gestures can generate much smoother scrolling events than the
discrete increments of mouse wheels. GDK3 supports this by means of a
new kind of scroll event, with direction GDK_SCROLL_SMOOTH and a
floating-point delta value. Added support for this; so in GTK3 mode,
you can now touchpad-scroll at a granularity of one line rather than
five, but in mouse tracking mode, scroll events are still grouped into
5-line chunks for purposes of turning them into escape sequences to
send to the server.
Apparently, if you don't specify GDK_SMOOTH_SCROLL_MASK in a widget's
event mask, then you don't receive "scroll_event" signals at all, even
of the non-smooth variety that was all GTK2 had. Hence, neither mouse
scroll wheels nor touchpad scroll gestures seem to generate any
response.
Adding GDK_SMOOTH_SCROLL_MASK brings the old scroll events back again,
so this is at least no worse than GTK2 was. But in GTK3 we _ought_ to
be able to do better still, by supporting smooth scrolling from
touchpads; this commit doesn't do that.
If you use the new 'plink -shareexists' feature, then on Unix at least
it's possible for the upstream to receive EPIPE, because the
downstream makes a test connection and immediately closes it, so that
upstream fails to write its version string.
This looks a bit ugly in the upstream's Event Log, so I'm making a
special case: an error of 'broken pipe' type, which occurs on a socket
from a connection sharing downstream, before we've received a version
string from that downstream, is treated as an unusual kind of normal
connection termination and not logged as an error.
A Plink invocation of the form 'plink -shareexists <session>' tests
for a currently live connection-sharing upstream for the session in
question. <session> can be any syntax you'd use with Plink to make the
actual connection (a host/port number, a bare saved session name,
-load, whatever).
I envisage this being useful for things like adaptive proxying - e.g.
if you want to connect to host A which you can't route to directly,
and you might already have a connection to either of hosts B or C
which are viable proxies, then you could write a proxy shell script
which checks whether you already have an upstream for B or C and goes
via whichever one is currently active.
Testing for the upstream's existence has to be done by actually
connecting to its socket, because on Unix the mere existence of a
Unix-domain socket file doesn't guarantee that there's a process
listening to it. So we make a test connection, and then immediately
disconnect; hence, that shows up in the upstream's event log.
When I abandoned GtkDialog for GtkWindow (in dc11417ae), I manually
added a horizontal GtkSeparator between the content and action areas.
Or rather, I tried to - but I forgot that gtk_box_pack_end works in
the opposite order, so that you have to add the bottom-most element
first and then the one you want to appear above it. So my separator
was below the action area, rather than between it and the content
area.
In GTK 3, it was impossible to resize the window smaller than it
started off, because the size request on the drawing area was taken as
a minimum. (I can't actually remember how the GTK 2 version doesn't
have this problem too.)
Fixed by instead setting the initial window size using
gtk_window_set_default_geometry() (having first set up the geometry
hints to reflect the character cell size).
On Windows, colons are illegal in filenames, because they're part of
the path syntax. But colons can appear in automatically constructed
log file names, if an IPv6 address is expanded from the &H placeholder.
Now we coerce any such illegal characters to '.', which is a bit of a
bodge but should at least cause a log file to be generated.
I noticed that Unix PSCP was unwantedly renaming downloaded files
which had a backslash in their names, because pscp.c's stripslashes()
treated \ as a path component separator, since it hadn't been modified
since PSCP ran on Windows only.
It also turns out that pscp.c, psftp.c and winsftp.c all had a
stripslashes(), and they didn't all have quite the same prototype. So
now there's one in winsftp.c and one in uxsftp.c, with appropriate
OS-dependent behaviour, and the ones in pscp.c and psftp.c are gone.
In any DRAWTYPE_CAIRO mode, we now do all our Cairo drawing to a Cairo
image surface which lives on the client; then we either blit directly
from that to the window (if we're in GTK3 mode, or GTK2 without
deprecated pieces of API), or else we blit from the Cairo surface to
the server-side pixmap and then from there to the actual window.
In DRAWTYPE_GDK mode, nothing much has changed: we still draw directly
to the server-side pixmap using the GDK drawing API, then blit from
there to the window. But there is one change, namely that the blit is
no longer done proactively - instead, we queue a redraw of the
affected rectangle, and wait until we're called back by the expose
handler.
The main aim of all this is to arrange that the only time we ever draw
to the real window is in response to expose/draw events. The
experimental GTK3 OS X port stopped working a week or two ago (I
presume in response to an OS update) with the symptoms that attempts
to draw on the window outside the context of a "draw" event handler
just didn't seem to work any more; this change fixes it.
In addition to that benefit, this change also has obvious performance
advantages in principle. No more expensive text rendering in response
to an expose event - just re-copy to the window from the bitmap we
already have, from wherever it's stored that's nearest.
Moreover, this seems to have fixed the significant performance problem
with X server-side fonts under GTK. I didn't expect _that_! I'd
guessed that the approach of downloading character bitmaps and
rendering them manually via Cairo was just inherently slow for some
reason. I've no real idea _why_ this change improves matters so much;
my best guess is that perhaps there's an overhead in each drawing
operation to a GDK Cairo surface, so we win by doing lots of
operations to a much faster image surface and then batching them up
into one GDK Cairo operation. But whyever it is, I'm certainly not
complaining!
(In fact, it now seems to be noticeably _faster_, at least on my usual
local X displays, to draw server-side fonts using that technique than
using the old GDK approach. I may yet decide to switch over...)
Plink sets standard input into nonblocking mode, meaning that read()
from fd 0 in an interactive context will typically return -1 EAGAIN.
But the prompt functions in uxcons.c, used for verifying SSH host keys
and suchlike, were doing an unguarded read() from fd 0, and then
panicking and aborting the session when they got EAGAIN.
Fixed by inventing a wrapper around read(2) which handles EAGAIN but
passes all other errors back to the caller. (Seemed slightly less
dangerous than the stateful alternative of temporarily re-blockifying
the file descriptor.)
When I introduced the KEY_EVENT_DIAGNOSTICS system last month in
commit 769600b22, I somehow didn't notice that it sat next to an
existing system of ifdefs labelled KEY_DEBUGGING, which did some
things worse but some things better.
Now I've expanded both of those into a fairly complete system of
diagnostics (keeping the newer name of KEY_EVENT_DIAGNOSTICS), and
made them use debug() rather than printf() so that in situations where
no standard output is available I can still retrieve the diagnostics
from debug.log.
I turned it into Shift-Return, because I was trying to find out why
the former didn't work on OS X and replaced it with something else
random to see if the code was even being reached. And then, like an
utter doofus, I committed that change as part of a50da0e30.
Rather than trying to get my existing hugely complicated X-style
clipboard code to somehow work with the Quartz GTK back end, I've
written an entirely new and much simpler alternative clipboard handler
usnig the higher-leve GtkClipboard interface. It assumes all clipboard
text can be converted to and from UTF-8 sensibly (which isn't a good
assumption on all front ends, but on OS X I think it's reasonable),
and it talks to GDK_SELECTION_CLIPBOARD rather than PRIMARY, which is
the only clipboard OS X has.
I had to do a fiddly thing to cope with the fact that each call to
gtk_clipboard_set_with_data caused a call to the clipboard clear
function left over from the previous set of data, so I had to avoid
mistaking that for a clipboard-clear for the _new_ data and
immediately deselecting it. I did that by allocating a distinct
placeholder object in memory for each instance of the copy operation,
so that I can tell whether a clipboard-clear is for the current copy
or a previous one.
This is only very basic support which demonstrates successful copying
and pasting is at least possible. For a sensible OS X implementation
we'll need a more believable means of generating a paste UI action
(it's quite easy to find a Mac on which neither Shift-Ins nor the
third mouse button even exists!). Also, after the trouble I had with
the clipboard-clear event, it's a bit annoying to find that it
_doesn't_ seem to get called when another application becomes the
clipboard owner. That may just be something we have to put up with, if
I can't find any reason why it's failing.
If I'm using Option as the Meta key, I want to suppress OS X GTK's
default behaviour of treating it as an AltGr-oid which changes the
keyval and Unicode translation of alphabetic keys. So on OS X I enable
a somewhat bodgy workaround which retranslates from the hardware
keycode as if the Option modifier had not been active at the time, and
use that as the character to prefix Esc to.
This is a bit nasty because I have to hardwire group = 0 in the call
to gdk_keymap_translate_keyboard_state(), whereas in principle what I
wanted was group = (whatever would have resulted from everything else
in the key event other than MOD1). However, in practice, they seem to
be the same, so this will do for the moment.
Personally I like using Command as the Esc-prefixing Meta key in
terminal sessions, because it occupies the same physical keyboard
position as the Alt key that I'm used to using on non-Macs. OS X
Terminal uses Option for that purpose (freeing up Command for the
conventional Mac keyboard shortcuts, of course), so I anticipate
differences of opinion.
Hence, here's a pair of OSX-specific config options which permit a
user to set either, or neither, or both of those modifier keys to
function as the terminal Meta key.
On OS X, apparently, we can't do termios setup on the pty master, so
instead we have to leave it until we've opened the slave fd in the
child process. That works on Linux too, so let's leave it here rather
than having another cumbersome ifdef.
In a UTF-8 pterm, it makes sense to set the IUTF8 flag (on systems
that have one) on the pty device, so that line editing will take
account of UTF-8 multibyte characters.
By retrieving characters' widths using get_extents and not
get_pixel_extents, we can spot when they're not actually an exact
multiple of a pixel, and avoid getting confused by the overall width
of a long string being off by up to a pixel per character.
The Pty that we created in pty_pre_init had its bufchain properly
initialised, but if that one didn't get created, then the one we
create in pty_init did not. Now both should go through the same init
routine.
Now I've moved align_label_left() into gtkmisc.c where gtkask.c can
get at it, we can use it to fix the alignment of the prompt label.
Also, use gtk_label_set_width_chars() to give the label a more or less
sensible width.
Several utility functions I've written over the last few weeks were in
rather random places because I didn't have a central gtkmisc.c to put
them in. Now I've got one, put them there!
They've now deprecated gtk_dialog_get_action_area, because they really
want a dialog box's action area to be filled with nothing but buttons
controlled by GTK which end the dialog with a response code. But we're
accustomed to putting all sorts of other things in our action area -
non-buttons, buttons that don't end the dialog, and sub-widgets that
do layout - and so I think it's no longer sensible to be trying to
coerce our use cases into GtkDialog.
Hence, I'm introducing a set of wrapper functions which equivocate
between a GtkDialog for GTK1 and GTK2, and a GtkWindow with a vbox in
it for GTK3, and I'll lay out the action area by hand.
(Not everything has sensible layout and margins in the new GTK3 system
yet, but I can sort that out later.)
Because the new functions are needed by gtkask.c, which doesn't link
against gtkdlg.c or include putty.h, I've put them in a new source
file and header file pair gtkmisc.[ch] which is common to gtkask and
the main GTK edifice.
When NULL appears in variadic argument lists, it should be cast to the
pointer type that the function will be expecting, because otherwise it
might end up as a type not even the same size as a pointer.
This is a much simpler way to display simple message-box type dialogs,
whose absence I've previously been working around by laboriously
constructing something in my usual style.
We were using it in the main config box to ensure everything expanded
on window resize, but in GTK3 that's the default anyway. And we were
using it to put padding around the edges of the font selector, which
is now done using the "margin" property.
I'm using a slightly more up-to-date GTK version for testing on MacOS,
and it's marked a few more functions as deprecated, among which is
gdk_color_parse(). So now parsing -fg and -bg options has to be done
by two different calls and an ugly #ifdef, depending on GTK version.
If we're not supporting server-side fonts, it's utterly silly to set
one as the default! Instead, we use Pango's guarantee that some
reasonably sensible monospaced font will be made available under the
name "Monospace", and use that at a reasonable default size of 12pt.
Using GTK to run on OS X is going to require several workarounds and
behaviour tweaks to be enabled at various points in the code, and it's
already getting cumbersome to remember what they all are to put on the
command line. Here's a central #define (OSX_GTK) that enables them all
in one go, and a configure option (--with-quartz) that sets it.
As part of this commit, I've also rearranged the #include order in the
GTK source files, so that they include unix.h (which now might be
where NOT_X_WINDOWS gets defined) before they test NOT_X_WINDOWS to
decide whether to include X11 headers.
The Quartz GDK back end, if you press (say) Ctrl-A, will generate a
GdkKeyEvent with keyval='a' and state=CONTROL, but it'll have a
translated string of "a" where the X back end would have returned
"\001". So we have to do our own translation, which fortunately isn't
hard.
OS X for some reason doesn't let my usual fcntl approach (wrapped in
nonblock()) work on pty masters - the fcntl(F_SETFL) fails, with the
(in this context) hilariously inappropriate error code ENOTTY. Work
around it by instead passing O_NONBLOCK to posix_openpt.
OS X dislikes us calling the setuid or setgid syscalls when not
privileged, even if we try to set ourselves to the _same_ uid/gid.
Since I don't anticipate this code needing to run setuid on OS X, and
since I do anticipate wanting to handle multiple ptys in a single
process so that pty_pre_init would be useless anyway, the simplest fix
seems to me to be just conditioning out the whole of pty_pre_init
completely.
On OS X GTK, it requests a preferred width that's way too large. I
think that's because that's based on its max_width_chars rather than
its width_chars (and I only set the latter). But I don't want to
actually reduce its max_width_chars, in case (either now or in a
future version) that causes it to actually refuse to take up all the
space it's allocated.
These should dump out all the important parts of the incoming
GdkEventKey, so that if keys aren't being translated right, it should
be possible to work out something about why not.
To enable: make CPPFLAGS="-DKEY_EVENT_DIAGNOSTICS"
These are a slightly cleaned-up version of the diagnostics I was using
to debug the layout problems in the GTK3 config box the other day. In
particular, if the box comes out far too wide - as I've just found out
that it also does when I compile the current state of the code against
OS X GTK3 - these diagnostics should provide enough information to
figure out which control is the limiting factor.
To enable: make CPPFLAGS="-DCOLUMNS_WIDTH_DIAGNOSTICS"
In shortcut_add(), when we add an underlined letter to a GtkLabel, we
were fetching the label's height before changing its text, and
restoring it afterwards. I've no idea why - I can see no difference
with and without the code.
That code's been there since 2003 without explanation. My best guess
is that it was working around a GTK bug of the day, but since no
difference is visible even in current GTK1, I think I'm just going to
remove it. If any problems show up later, I can put it back, with an
actual comment!
The whole of get_label_text_dimensions() should have been outside the
GTK 2 ifdef; I'd left a gtk_label_set_width_chars() unconditional; and
GDK1's gdk_window_set_background() lacks a const in its prototype.
Serves me right for not test-compiling in all three versions!
My trickery in GTK2 to start with some branches of the tree collapsed
but give the widget all the width it will need when they open later
was not working in GTK3, for the same reason I've needed several other
fixes recently: just after creation, GTK3 widgets report their
preferred size as zero.
Fixed by doing basically the same trick I was doing in GTK2, but
deferring it until the "map" event happens later on.
This was another piece of code that determined text size by
instantiating a GtkLabel and asking for its size, which I had to fix
in gtkfont.c recently because that strategy doesn't work in GTK3.
Replaced the implementation of string_width() with a call to the
function I added in gtkfont.c, and now dialog boxes which depend on
that for their width measurement (e.g. the one in reallyclose()) don't
come out in silly sizes on GTK3 any more.
In cases where two controls sit alongside one another such as a label
alongside an edit box, I've previously been arranging for them to be
vertically centred by fiddling with the size request and alignment of
what I assume will be the shorter control. But now I've written
columns_force_same_height(), that's a much easier approach, and it's
also compatible with GTK3 without using a deprecated method; so this
change switches over all vertical centring to doing it that way.
Also, while I'm here, I noticed that the edit box and button of
CTRL_FILESELECT / CTRL_FONTSELECT were not vertically centred, and
since it's now really easy to make sure they are, I've added another
use of columns_force_same_height() there.
This forces two child widgets of a Columns to occupy the same amount
of vertical space, and if one is really shorter than the other,
vertically centres it in the extra space.
This is an obviously reusable loop over cols->children looking for a
widget, which I'm about to use a couple more times so it seems worth
pulling it out into its own helper function.
gtk_window_resize_to_geometry() allows us to make use of the already-
set-up WM resize hints to immediately figure out how to resize the
window to a particular character cell size, and hence makes a much
simpler implementation of request_resize() than the previous hackery.
Now that I've got the main calculation code separated from the GTK2
size_request and size_allocate top-level methods, I can introduce a
completely different set of GTK3 top-level methods, which run the same
underlying calculations but based on different width and height
information.
So now we do proper height-for-width layout, as you can see if you
flip the PuTTY config box to a pane with a wrapping label on it (e.g.
Fonts or Logging) and resize the window horizontally. Where the GTK2
config box just left the wrapped text label at its original size, and
the GTK3 one before this change would reflow the text but without
changing the label's height, now the text reflows and the controls
below it move up and down when the number of lines of wrapped text
changes.
(As far as I'm concerned, that's a nice demo of GTK3's new abilities
but not a critically important UI feature for this app. The more
important point is that switching to the modern layout model removes
one of the remaining uses of the deprecated gtk_widget_size_request.)
Previously, columns_size_request and columns_size_allocate would each
loop over all the widgets doing computations for both width and
height. Now I've separated out the width parts from the height parts,
and moved both out into four new functions, so that the top-level
columns_size_request and columns_size_allocate are just wrappers that
call the new functions and plumb size and position information between
them and GTK.
Actual functionality should be unchanged by this patch.
Profiling reveals that pterm in Pango rendering mode uses an absurd
amount of CPU when it's not even actually _drawing_ the text, because
of all the calls to pango_layout_get_pixel_extents() while
pangofont_draw_text tries to work out which characters it can safely
draw as part of a long string. Caching the results speeds things up
greatly.
If you're trying to arrange that an array size is large enough for
element n to exist, and you also want to round it up to the next
multiple of 0x100, you must set the size to (n + 0x100) & ~0xFF, and
not (n + 0xFF) & ~0xFF. Put another way, the number you have to round
up is not n, but the minimum size n+1 that causes array[n] to exist.
If you open a pterm on a different display via the --display
command-line option rather than by setting $DISPLAY, I think (and
other terminals seem to agree) that it's sensible to set $DISPLAY
anyway for processes running inside the terminal.
This replaces the old GtkColorSelectionDialog, and has the convenience
advantage that the actual chooser (with all the 'set colour', 'get
colour' methods) and the containing dialog box are now the same object
implementing multiple interfaces, so I don't keep having to call 'get
me the underlying chooser for this dialog' accessors. Also you now
hook into both the OK and Cancel buttons (and all other response
codes) at the same time with a single event.
GtkTable is deprecated; the way of the future is GtkGrid, in which you
don't have to specify the number of rows/columns in advance (it's
worked out dynamically by observing what row/column numbers you
actually attached anything to), and also you handle expansion
behaviour by setting the "hexpand", "vexpand" or "expand" properties
on the child widgets rather than setting flags in the container.
The new way is gdk_display_get_name(gdk_display_get_default()), which
returns a const char * rather than a char *, so I've also had to
fiddle with the prototype and call sites of get_x_display().
(Also included gtkcompat.h into uxputty.c, since that wanted to call
gdk_get_display() but didn't previously include it.)
In GTK3, GtkBox, GtkScrollbar and GtkSeparator are all single classes
with a GtkOrientation parameter, whereas in GTK2 the horizontal and
vertical versions were trivial subclasses of them. Hence, the old
constructors are now deprecated, but _only_ the constructors are
affected, since after constructing one you always used methods of the
superclass on it anyway.
Rather than faff about with an ifdef at every call site, I've just put
some wrapper macros in gtkcompat.h to make it easy to keep this code
similar between all supported GTK versions.
In the case where we deselect the previously selected font (e.g.
because we've just changed the filter settings to remove it from the
list), we were leaving the preview pane in its previous state, which
is fine in GTK2 when it just carries on displaying the last thing
drawn to the backing pixmap but goes wrong in GTK3 where we still have
to actually respond to draw events.
But it makes more conceptual sense anyway to actually empty the
preview pane when no font is selected, so now we do that. So now
unifontsel_draw_preview_text() is called from unifontsel_deselect(),
and also the preview-drawing code will still draw the background
rectangle regardless of whether font != NULL.
The call to gtk_list_store_clear() in unifontsel_setup_familylist()
was causing a call to family_changed() via the GTK signal system,
which didn't happen in GTK2. family_changed() in turn was calling
unifontsel_select_font(), which got confused when the tree model
didn't match reality, and tried to access a bogus tree iterator.
This is easily fixed by using the existing fs->inhibit_response flag,
which prevents us responding to GTK events when we know they were
generated by our own fiddling about with the data; it's just that we
never needed to set it in unifontsel_setup_familylist() before.
Also, added a check of the return value from the key get_iter call in
unifontsel_select_font(), so that it'll at least fail an assertion
rather than actually trying to access bogus memory. But that operation
_should_ still always succeed, and if it doesn't, it's probably a sign
that we need another use of fs->inhibit_response.
It turns out that in GTK3, if you instantiate a GtkLabel and
immediately try to find out its preferred size, you get back zero for
both dimensions. Presumably none of that gets figured out properly
until the widget is displayed, or some such.
However, you can retrieve the PangoLayout from the label immediately
and ask Pango for the dimensions of that. That seems like a bit of a
bodge, but it works! The GTK3 unifont selector now comes out with all
the interface elements in sensible sizes - in particular, the preview
drawing area now has non-zero height.
The config box setup code wants a lot of very narrow edit boxes, so it
decreases their minimum width from the GTK default of 150 pixels. In
GTK 3, we have to do this using gtk_entry_set_width_chars() rather
than gtk_widget_set_size_request() as we were previously using, or it
won't work.
This change by itself seems to restore the GTK3 config box to a
sensible layout, in spite of the fact that my Columns layout class is
still doing all its size allocation the GTK2 way, with no attention
paid to the shiny new GTK3 height-for-width system.
The entire concept has gone away in GTK3, which assumes that everyone
is now using modern true-colour video modes and so there's no longer
any reason you shouldn't just casually make up any RGB triple you like
without bothering to ask the display system's permission.
GDK3 now spells both of those as GDK_WINDOW_XID. (Of course 'drawable'
is no longer a relevant concept in GDK3, since pixmaps are no longer
supported and so all drawables are just windows.) We keep backwards
compatibility, of course.
It's been replaced by a new pair of methods get_preferred_width and
get_preferred_height. For the moment, I've followed the porting
guide's suggestion of keeping the old size_request function as an
underlying implementation and having each of those methods just return
one of its outputs. The results are ugly, but it'll compile and run,
which is a start.
This replaces GTK 1/2's "expose_event", and provides a ready-made
cairo_t to do the drawing with. My previous work has already separated
all constructions of a cairo_t from the subsequent drawing with it, so
the new draw event handlers just have to call the latter without the
former.
This is the new recommended approach since gdk_input_{add,remove} were
deprecated (and, honestly, seems a lot more sensible - why on earth
would those functions have lived in *GDK* of all places?). The old
implementation is preserved under ifdef for GTK1.
This was the last of the GDK deprecated functions to go! So GTK PuTTY
now compiles cleanly with -DGDK_DISABLE_DEPRECATED in addition to all
the other precautionary flags (though if you do that, you disable GDK
rendering, which greatly slows down server-side font handling). This
completes the GTK2-compatible preparation phase of the GTK 3 migration
guide.
In case a front end needs to store more than an integer id to be
returned to uxsel_input_remove, we now return a pointer to a
frontend-defined structure.
GTK is deprecating the use of gdk_window_set_icon(), in favour of a
method that doesn't have to drop down to the GDK level at all (and
also doesn't use a pixmap). No reason not to use that instead.
I've just noticed the comment in gtkfont.c that said wouldn't it be
nice to find a way to avoid the GDK pixmap-stretching code when using
Pango fonts. We now do support this, but we support it in gtkwin.c
rather than gtkfont.c - because we do it using a Cairo transformation
matrix, so it still takes place at the level above Pango rather than
in Pango proper. (I never did find out whether Pango itself included
facilities to arbitrarily stretch a font.)
Hence, this comment is useless now. Discard.
We still don't actually support more than one X display active at
once, so it's sufficient to replace every call to that macro with
GDK_DISPLAY_XDISPLAY(gdk_display_get_default()).
We won't be able to use them in GTK3, or when compiling with GTK2 and
-DGDK_DISABLE_DEPRECATED.
This applies to the one we use for the main terminal window, and also
the small one we use for the preview pane in the unified font selector.
Now it's got an inner half that does actual drawing given a draw
context, and an outer half that sets up and tears down the draw
context. Sooner or later the inner half will need calling
independently of the outer, because GTK3's draw event will provide a
ready-made cairo_t.
A small bug in yesterday's work: since in Cairo mode
draw_stretch_before changes the transformation matrix, if we do it
before calling draw_clip then the clip region will be interpreted in
the transformed coordinates.
This caused a subtle display bug in yesterday's commit: drawing one
half of double-height text would have drawn _both_ halves of it on to
the window's backing pixmap, but only copied the correct half on to
the window proper - but the overdrawing on the pixmap would have shown
up if the window was hidden and re-exposed.
We were previously building our own mouse pointers out of pixmaps,
having first drawn characters from the X server standard font 'cursor'
on to those pixmaps, giving an effect almost exactly the same as just
calling gdk_cursor_new(some constant) except that we got to choose the
foreground and background colours of the resulting pointers.
But it's not clear why we needed to do that! In both GTK1 and GTK2 as
of my current testing, the standard colours appear to be just what I
wanted anyway (white pointer with black outline). The previous
implementation (and commit comment) was written in 2002, so perhaps it
was working around a GTK1 bug of the time.
So I've removed it completely, and replaced it with simple calls to
gdk_cursor_new (plus a workaround for GTK1's lack of GDK_BLANK_CURSOR,
but that's still much simpler than the previous code). If anyone does
report a colour problem, I may have to go back to doing something
clever, but if I can possibly arrange it, I'll want to do it by some
other technique, probably (as suggested in a comment in the previous
implementation) getting the underlying X cursor id and calling
XRecolorCursor.
We're going to have to use Cairo in the GTK3 port, because that's all
GTK3 supports; but we still need old-style GDK for GTK1 support, and
also for performance reasons in GTK2 (see below). Hence, this change
completely restructures GTK PuTTY's drawing code so that there's a
central 'drawing context' structure which contains a type code
indicating GDK or Cairo, and then either some GDK gubbins or some
Cairo gubbins as appropriate; all actual drawing is abstracted through
a set of routines which test the type code in that structure and do
one thing or another. And because the type code is tested at run time,
both sets of drawing primitives can be compiled in at once, and where
possible, they will be.
X server-side bitmap fonts are still supported in the Cairo world, but
because Cairo drawing is entirely client-side, they have to work by
cheekily downloading each glyph bitmap from the server when it's first
needed, and building up a client-side cache of 'cairo_surface_t's
containing the bitmaps with which we then draw on the window. This
technique works, but it's rather slow; hence, even in GTK2, we keep
the GDK drawing back end compiled in, and switch over to it when the
main selected font is a bitmap one.
One visible effect of the new Cairo routines is in the double-width
and double-height text you can get by sending ESC # 3, ESC # 4 and
ESC # 6 escape sequences. In GDK, that's always been done by a really
horrible process of manually scaling the bitmap, server-side, column
by column and row by row, causing each pixel to be exactly doubled or
quadrupled. But in Cairo, we can just set a transformation matrix, and
then that takes effect _before_ the scalable fonts are rendered - so
the results are visibly nicer, and use all the available resolution.
(Sadly, if you're using a server-side bitmap font as your primary one,
then the GDK backend will be selected for all drawing in the terminal
as a whole - so in that situation, even fallback characters absent
from the primary font and rendered by Pango will get the old GDK
scaling treatment. It's only if your main font is scalable, so that
the Cairo backend is selected, that DW/DH characters will come out
looking nice.)
I was tacitly assuming that mfont->fallback would always be non-NULL,
which is true in a world containing Pango, but untrue in GTK1 when
Pango isn't there. In that situation we fall back to just omitting the
characters that would be displayed in the fallback font, on the
grounds that that's better than dereferencing through a NULL vtable.
GTK 2 has deprecated it and provided no replacement; a bug tracker
entry I found on the subject suggested that it was functionality that
didn't really belong in GTK, and glib ought to provide a replacement
instead, which would be a perfectly fine thing to suggest if they had
waited for glib to get round to doing so *before* throwing out a
function people were actually using. Sigh.
Anyway, it turns out that subsidiary invocations of gtk_main() don't
happen inside GTK as far as I can see, so all I need to do is to make
sure my own invocations of gtk_main() are followed by a cleanup
function which runs any quit functions that I've registered.
That was the last deprecated GTK function, so we now build cleanly
with -DGTK_DISABLE_DEPRECATED. (But, as mentioned a couple of commits
ago, we still don't build with -DGDK_DISABLE_DEPRECATED, because that
has migrating to Cairo drawing as a prerequisite.)
Now that I've got a general place to centralise handling of at least
the simple differences between GTK 1 and 2, I should use it wherever
possible. So this commit removes just a small number of ifdefs which
are either obsoleted by definitions already in gtkcompat.h (like
set_size_request vs set_usize), or can easily be replaced by adding
another (e.g. gtk_color_selection_set_has_opacity_control).
Building with -DGTK_DISABLE_DEPRECATED, we now suffer only one compile
failure, for the use of gtk_quit_add() in idle_toplevel_callback_func.
That function is apparently removed with no replacement in GTK 3, so
I'll need to find a completely different approach to getting toplevel
callbacks to run only in the outermost instance of gtk_main().
Also, this change doesn't do anything about the use of *GDK*
deprecated functions, because those include the entire family of
old-style drawing functions - i.e. the only way to build cleanly with
-DGDK_DISABLE_DEPRECATED will be to switch to Cairo drawing.
On GTK versions where it's available, this is a much nicer way of
handling the -geometry command-line option, since not only do we get
all the faffing about with gravity for free, it also automatically
sets the user-position WM hints.
I've put in a special #define to control this selection, in case I
decide that for reasons of taste I'd prefer to switch back to
GtkFileSelection in GTK2 which supports both!
Replaces the deprecated gtk_color_selection_set_color() which took an
array of four doubles (RGBA), and instead takes a 'GdkColor' struct
containing four 16-bit integers.
For GTK1, we still have to retain the original version.
All the things like GtkType, GtkObject, gtk_signal_connect and so on
should now consistently have the new-style glib names like GType,
GObject, g_signal_connect, etc.
A major aim of introducing GTK 3 support is to permit compiling for
non-X11 platforms that GTK 3 supports, so I'm going to need to be able
to build as a pure GTK application with no use of X11 internals.
Naturally, I don't intend to stop supporting the hybrid GTK+X11 mode
in which X server-side bitmap fonts are available.
Use of X11 can be removed by compiling with -DNOT_X_WINDOWS. That's
the same compatibility flag that was already used by the unfinished OS
X port to disable the X-specific parts of uxpty.c; now it just applies
to more source files.
(There's no 'configure' option to set this flag at present. I haven't
worked out whether we'll need one yet.)
GTK 2 doesn't _documentedly_ provide a helpful compile option to let
us check this one in advance of GTK 3, but you can fake one anyway by
compiling with -D__GDK_KEYSYMS_COMPAT_H__, so that gdkkeysyms-compat.h
will believe that it's already been included :-) We now build cleanly
under GTK 2 with that predefine.
This is the first of several cleanup steps recommended by the GTK 2->3
migration guide.
I intend to begin work towards compatibility with GTK 3, but without
breaking GTK 2 and even GTK 1 compatibility in the process; GTK 2 is
still useful to _me_ (not least because it permits much easier support
of old-style server-side X11 fonts), and I recall hearing a rumour
that at least one kind of strange system can only run GTK 1, so for
the moment I don't intend to stop supporting either.
Including gdkkeysyms.h is not optional in GTK 2, because gdk.h does
not include it. In GTK 3 it does, so we don't explicitly reinclude it
ourselves.
We now build cleanly in GTK2 with -DGTK_DISABLE_SINGLE_INCLUDES. (But
that doesn't say much, because we did already! Apparently gdkkeysyms.h
was a special case which that #define didn't forbid.)
This is less than ideal - passphrase input now happens in ISO 8859-1,
and the passphrase prompt window is neither centred nor always-on-top.
But it basically works, and restores bare-minimum GTK 1 support to the
codebase as a whole.
If you're counting up to ms_limit in steps of ms_step, it's silly to
add ms_step at the end of the loop body _and_ increment the loop
variable by 1 in the loop header. I must have been half asleep.
The general plan is that if PuTTY knows a host key for a server, it
should preferentially ask for the same type of key so that there's some
chance of actually getting the same key again. This should mean that
when a server (or PuTTY) adds a new host key type, PuTTY doesn't
gratuitously switch to that key type and then warn the user about an
unrecognised key.
An unguarded write() in the dputs function caused gcc -Werror to fail
to compile. I'm confused that this hasn't bitten me before, though -
obviously normal builds of PuTTY condition out the faulty code, but
_surely_ this can't be the first time I've enabled the developer
diagnostics since gcc started complaining about unchecked syscall
returns!
When anyone connects to a PuTTY tool's listening socket - whether it's
a user of a local->remote port forwarding, a connection-sharing
downstream or a client of Pageant - we'd like to log as much
information as we can find out about where the connection came from.
To that end, I've implemented a function sk_peer_info() in the socket
abstraction, which returns a freeform text string as best it can (or
NULL, if it can't get anything at all) describing the thing at the
other end of the connection. For TCP connections, this is done using
getpeername() to get an IP address and port in the obvious way; for
Unix-domain sockets, we attempt SO_PEERCRED (conditionalised on some
moderately hairy autoconfery) to get the pid and owner of the peer. I
haven't implemented anything for Windows named pipes, but I will if I
hear of anything useful.
Caused an embarrassing failure just now trying to run the test program
from a command prompt - I had Return still held down by the time it
started up, and my release of it immediately terminated input :-)
Having found a lot of unfixed constness issues in recent development,
I thought perhaps it was time to get proactive, so I compiled the
whole codebase with -Wwrite-strings. That turned up a huge load of
const problems, which I've fixed in this commit: the Unix build now
goes cleanly through with -Wwrite-strings, and the Windows build is as
close as I could get it (there are some lingering issues due to
occasional Windows API functions like AcquireCredentialsHandle not
having the right constness).
Notable fallout beyond the purely mechanical changing of types:
- the stuff saved by cmdline_save_param() is now explicitly
dupstr()ed, and freed in cmdline_run_saved.
- I couldn't make both string arguments to cmdline_process_param()
const, because it intentionally writes to one of them in the case
where it's the argument to -pw (in the vain hope of being at least
slightly friendly to 'ps'), so elsewhere I had to temporarily
dupstr() something for the sake of passing it to that function
- I had to invent a silly parallel version of const_cmp() so I could
pass const string literals in to lookup functions.
- stripslashes() in pscp.c and psftp.c has the annoying strchr nature
All the name strings in ssh_cipher, ssh_mac, ssh_hash, ssh_signkey
point to compile-time string literals, hence should obviously be const
char *.
Most of these const-correctness patches are just a mechanical job of
adding a 'const' in the one place you need it right now, and then
chasing the implications through the code adding further consts until
it compiles. But this one has actually shown up a bug: the 'algorithm'
output parameter in ssh2_userkey_loadpub was sometimes returning a
pointer to a string literal, and sometimes a pointer to dynamically
allocated memory, so callers were forced to either sometimes leak
memory or sometimes free a bad thing. Now it's consistently
dynamically allocated, and should be freed everywhere too.
I've written my own analogue of OpenSSH's ssh-askpass. At the moment,
it's contained inside Pageant proper, though it could easily be
compiled into a standalone binary as well or instead.
Unlike OpenSSH's version, I don't use a GTK edit box; instead I just
process key events myself and append them to a buffer. The big
advantage of doing this is that I can arrange for ^W and ^U to
function as they do in terminal line editing, i.e. delete a word or
delete the whole line.
^W in particular is really valuable when typing a multiple-word
passphrase unseen. If you feel yourself making the kind of typo in
which you're not sure if you pressed six keys or just five, you can
hit ^W and restart just that word, without either having to go right
back to the beginning or carry on and see if you feel lucky.
A delete-word function would of course be an information leak in even
an obscured edit box (displaying a blob per character), so instead I
give a visual acknowledgment of keypresses by a more ad-hoc means: I
display three lights in the box, and every meaningful keypress turns
off the currently active one and instead turns on a randomly selected
one of the others. (So the lit light doesn't even indicate _mod 3_ how
many keys have been pressed.)
I had freed the comment string coming back from pageant_add_keyfile,
but not NULLed out the pointer, so that the cleanup code at the end of
the function would have freed it again.
I've decided against implementing an option exactly analogous to
'ssh-add -L' (printing the full public key of everything in the
agent). Instead, you can identify a specific key to display in full,
by any of the same means -d lets you use, and then print it in either
of the public key formats we support.
Unlike ssh-add, we can identify the key by its comment or by a prefix
of its fingerprint as well as using a public key file on disk. The
string given as an argument to -d is interpreted as whichever of those
things matches; disambiguating prefixes are available if needed.
You can now load keys at Pageant init time, by putting the key file
names as bare arguments on the command line, e.g. 'pageant -T key.ppk'
or 'pageant key.ppk --exec some command'; also, 'pageant -a key.ppk'
behaves more or less like ssh-add, contacting an existing agent to add
the key.
The askpass() function currently supports terminal-based prompting
only. X11 askpass is yet to be implemented.
This brings in the code we'll need to request passphrases from the
terminal, and to talk to an existing SSH agent as a client.
Adding uxcons.c required adjusting the set of stub functions in
uxpgnt.c: uxcons.c removed the need for several, but added one of its
own (log_eventlog). A net win, though.
I've moved the setup and running of the actual agent server into
run_agent(), so that main() is now only command-line parsing and
validation. We recognise a collection of new command-line options for
talking to an existing agent as a client (analogous to ssh-add), which
go to a new run_client() function, but I haven't filled in that
function itself yet.
This is only intended for use in Unix Pageant; for any application
that's actually trying to get something else useful done at the same
time as the agent request is pending, it's much more sensible to use
the more rigorous existing approach of requesting a callback once the
agent request is answered.
Adding this mode is the easiest way to allow Unix Pageant's
command-line key loading to work, but it doesn't solve the underlying
problem that the supposedly cross-platform pageant_add_keyfile will
not work on a platform where we really _are_ constrained to do agent
requests asynchronously (perhaps because we're a GUI app in some
system that doesn't let us control our own top-level event loop).
If and when that situation arises, I'll have no choice but to turn
pageant_add_keyfile and friends (specifically, any function in
pageant.c that calls agent_query) into coroutine-structured functions,
and have clients call them repeatedly until they return 'finished'.
But for now, this is a lot easier!
Now --exec instantly terminates option processing, by treating
everything after it as the command. This means it doesn't matter if
the --exec command word looks like another option, and it also means
we can simplify the handling of real non-option argument words, when I
get round to adding some for loading keys.
This is intended to be a useful mode when you want to run an ssh agent
in a terminal session with no X11 available. You just execute a
command along the lines of eval $(pageant -T), and then Pageant will
run in the background for the rest of that terminal session - and when
the terminal session ends, so that Pageant loses its controlling tty,
it will take that as the signal to shut down. So, no need to manually
kill it, and unlike 'pageant --exec $SHELL', you can also do this half
way through a session if you don't realise until later that you need
an SSH agent, without losing any shell command history or other shell
context that you've accumulated so far in the session.
Unfortunately, I haven't been able to find any reliable way to
actually implement this -T mode, short of having Pageant wake up at
regular intervals and try to open /dev/tty to see if it's still there.
I had hoped that I could arrange to reliably get SIGHUP, or select on
/dev/tty for exceptional conditions, or some such, but nothing I've
tried along those lines seems to work.
The last use of it, to store the contents of the saved session name
edit box, was removed nearly two years ago in svn r9923 and replaced
by ctrl_alloc_with_free. The mechanism has been unused ever since
then, and I suspect any further uses of it would be a bad idea for the
same reasons, so let's get rid of it.
The memory dangling off ssd->sesslist should be freed when ssd itself
goes away, and the font settings ctrlset we delete in gtkcfg.c should
be freed as well once it's been removed from its containing array.
Thanks to Ranjini Aravind for pointing these out.
I've moved the listening socket setup back to before the lifetime
preparations, so in particular we find out that we couldn't bind to
the socket _before_ we fork. The only part that really needed to come
after lifetime setup was the logging setup, so that's now a separate
function called later.
Also, the random exit(0)s in silly places like x11_closing have turned
into setting a time_to_die flag, so that all clean exits funnel back
to the end of main() which at least tries to tidy up a bit afterwards.
(Finally, fixed a small bug in testing the return value of waitpid(),
which only showed up once we didn't exit(0) after the first wait.
Ahem.)
Now it actually logs all its requests and responses, the fingerprints
of keys mentioned in all messages, and so on.
I've also added the -v option, which causes Pageant in any mode to
direct that logging information to standard error. In --debug mode,
however, the logging output goes to standard output instead (because
when debugging, that information changes from a side effect to the
thing you actually wanted in the first place :-).
An internal tweak: the logging functions now take a va_list rather
than an actual variadic argument list, so that I can pass it through
several functions.
LIFE_EXEC is already dealt with, and I forgot to take out the comment
reminding me to do it, ahem.
The LIFE_PARENT mentioned in the same comment was an idea I had but
couldn't think of a way to make it work: if you have a terminal-only
shell session in which you want to eval $(ssh-agent), then it's
annoying and fragile to have to remember to kill the agent when you
log out, so you'd like it to automatically tie its lifetime to that of
the shell from which you invoked it. Unfortunately, I don't know of
any way to do that without race conditions. (E.g. if only pageant
didn't fork, then it could poll its own ppid until it became 1 - but
the child process would find it was 1 already.)
This is much more like ssh-agent than the Windows version is - it sets
SSH_AUTH_SOCK and SSH_AGENT_PID as its means of being found by other
processes, rather than Windows Pageant's approach of establishing
itself in a well-known location. But the actual agent code is the same
as Windows Pageant.
For the moment, this is an experimental utility and I don't expect it
to be useful to many people; its immediate use to me is that it
provides a way to test and debug the agent code on Unix, and also to
use the agent interface as a convenient way to exercise public key
functions I want to debug. And of course it means I can be constantly
using and testing my own code, on whatever platform I happen to be
using. In the further future, I have a list of possible features I
might add to it, but I don't know which ones I'll decide are
worthwhile.
One feature I've already put in is a wider range of lifetime
management options than ssh-agent: the -X mode causes Pageant to make
a connection to your X display, and automatically terminate when that
connection closes, so that it has the same lifetime as your X session
without having to do the cumbersome trick of exec()ing the subsequent
session-management process.
It tries to use the local username as the remote username if it has no
better ideas, but the presence of Default Settings would defeat this,
even if it had no username set. Reported by Jonathan Amery.
If we use getaddrinfo to translate the source IP address into a
sockaddr, then we need to freeaddrinfo the returned data later. Patch
due to Tim Kosse.
I don't think anyone has ever actually called it that, colloquially
_or_ formally, and if anyone ever did (in a bug report, say) I'd
probably have to stop and think to work out what they meant. It's
universally called Plink, and should be officially so as well :-)
