[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.
