- Jun 14, 2020
-
-
Simon Tatham authored
A user reported another situation in which that assertion can fail: if you paste text into the terminal that consists 100% of characters not available in the CONF_line_codepage character set, then the translation step generates the empty string as output, and that gets passed to ldisc_send by term_paste without checking. Previous bugs of this kind (see commits 4634cd47 and 43a63019) were fixed by adding a check before calling ldisc_send. But in commit 4634cd47 I said that probably at some point the right fix would be to remove the assertion in ldisc_send itself, so that passing len==0 becomes legal. (The assertion was there in the first place to catch cases where len==0 was used with its obsolete special meaning of signalling 'please update your status'.) Well, I think it's finally time. The assertion is removed: it's now legal again to call ldisc_send with an empty buffer, and its meaning is no longer the archaic special thing, but the trivial one of sending zero characters through the line discipline. (cherry picked from commit cd3e917f)
-
Simon Tatham authored
A user reported another situation in which that assertion can fail: if you paste text into the terminal that consists 100% of characters not available in the CONF_line_codepage character set, then the translation step generates the empty string as output, and that gets passed to ldisc_send by term_paste without checking. Previous bugs of this kind (see commits 4634cd47 and 43a63019) were fixed by adding a check before calling ldisc_send. But in commit 4634cd47 I said that probably at some point the right fix would be to remove the assertion in ldisc_send itself, so that passing len==0 becomes legal. (The assertion was there in the first place to catch cases where len==0 was used with its obsolete special meaning of signalling 'please update your status'.) Well, I think it's finally time. The assertion is removed: it's now legal again to call ldisc_send with an empty buffer, and its meaning is no longer the archaic special thing, but the trivial one of sending zero characters through the line discipline.
-
- Sep 08, 2019
-
-
Simon Tatham authored
The number of people has been steadily increasing who read our source code with an editor that thinks tab stops are 4 spaces apart, as opposed to the traditional tty-derived 8 that the PuTTY code expects. So I've been wondering for ages about just fixing it, and switching to a spaces-only policy throughout the code. And I recently found out about 'git blame -w', which should make this change not too disruptive for the purposes of source-control archaeology; so perhaps now is the time. While I'm at it, I've also taken the opportunity to remove all the trailing spaces from source lines (on the basis that git dislikes them, and is the only thing that seems to have a strong opinion one way or the other). Apologies to anyone downstream of this code who has complicated patch sets to rebase past this change. I don't intend it to be needed again.
-
- Jun 18, 2019
-
-
Simon Tatham authored
Having decided that the terminal's local echo setting shouldn't be allowed to propagate through to termios, I think the local edit setting shouldn't either. Also, no other terminal emulator I know seems to implement this sequence, and if you enable it, things get very confused in general. I think it's generally better off absent; if somebody turns out to have been using it, then we'll at least be able to find out what it's good for.
-
- Feb 28, 2019
-
-
Simon Tatham authored
The idea of these is that they centralise the common idiom along the lines of if (logical_array_len >= physical_array_size) { physical_array_size = logical_array_len * 5 / 4 + 256; array = sresize(array, physical_array_size, ElementType); } which happens at a zillion call sites throughout this code base, with different random choices of the geometric factor and additive constant, sometimes forgetting them completely, and generally doing a lot of repeated work. The new macro sgrowarray(array,size,n) has the semantics: here are the array pointer and its physical size for you to modify, now please ensure that the nth element exists, so I can write into it. And sgrowarrayn(array,size,n,m) is the same except that it ensures that the array has size at least n+m (so sgrowarray is just the special case where m=1). Now that this is a single centralised implementation that will be used everywhere, I've also gone to more effort in the implementation, with careful overflow checks that would have been painful to put at all the previous call sites. This commit also switches over every use of sresize(), apart from a few where I really didn't think it would gain anything. A consequence of that is that a lot of array-size variables have to have their types changed to size_t, because the macros require that (they address-take the size to pass to the underlying function).
-
- Nov 03, 2018
-
-
Simon Tatham authored
My normal habit these days, in new code, is to treat int and bool as _almost_ completely separate types. I'm still willing to use C's implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine, no need to spell it out as blob.len != 0), but generally, if a variable is going to be conceptually a boolean, I like to declare it bool and assign to it using 'true' or 'false' rather than 0 or 1. PuTTY is an exception, because it predates the C99 bool, and I've stuck to its existing coding style even when adding new code to it. But it's been annoying me more and more, so now that I've decided C99 bool is an acceptable thing to require from our toolchain in the first place, here's a quite thorough trawl through the source doing 'boolification'. Many variables and function parameters are now typed as bool rather than int; many assignments of 0 or 1 to those variables are now spelled 'true' or 'false'. I managed this thorough conversion with the help of a custom clang plugin that I wrote to trawl the AST and apply heuristics to point out where things might want changing. So I've even managed to do a decent job on parts of the code I haven't looked at in years! To make the plugin's work easier, I pushed platform front ends generally in the direction of using standard 'bool' in preference to platform-specific boolean types like Windows BOOL or GTK's gboolean; I've left the platform booleans in places they _have_ to be for the platform APIs to work right, but variables only used by my own code have been converted wherever I found them. In a few places there are int values that look very like booleans in _most_ of the places they're used, but have a rarely-used third value, or a distinction between different nonzero values that most users don't care about. In these cases, I've _removed_ uses of 'true' and 'false' for the return values, to emphasise that there's something more subtle going on than a simple boolean answer: - the 'multisel' field in dialog.h's list box structure, for which the GTK front end in particular recognises a difference between 1 and 2 but nearly everything else treats as boolean - the 'urgent' parameter to plug_receive, where 1 vs 2 tells you something about the specific location of the urgent pointer, but most clients only care about 0 vs 'something nonzero' - the return value of wc_match, where -1 indicates a syntax error in the wildcard. - the return values from SSH-1 RSA-key loading functions, which use -1 for 'wrong passphrase' and 0 for all other failures (so any caller which already knows it's not loading an _encrypted private_ key can treat them as boolean) - term->esc_query, and the 'query' parameter in toggle_mode in terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h, but can also hold -1 for some other intervening character that we don't support. In a few places there's an integer that I haven't turned into a bool even though it really _can_ only take values 0 or 1 (and, as above, tried to make the call sites consistent in not calling those values true and false), on the grounds that I thought it would make it more confusing to imply that the 0 value was in some sense 'negative' or bad and the 1 positive or good: - the return value of plug_accepting uses the POSIXish convention of 0=success and nonzero=error; I think if I made it bool then I'd also want to reverse its sense, and that's a job for a separate piece of work. - the 'screen' parameter to lineptr() in terminal.c, where 0 and 1 represent the default and alternate screens. There's no obvious reason why one of those should be considered 'true' or 'positive' or 'success' - they're just indices - so I've left it as int. ssh_scp_recv had particularly confusing semantics for its previous int return value: its call sites used '<= 0' to check for error, but it never actually returned a negative number, just 0 or 1. Now the function and its call sites agree that it's a bool. In a couple of places I've renamed variables called 'ret', because I don't like that name any more - it's unclear whether it means the return value (in preparation) for the _containing_ function or the return value received from a subroutine call, and occasionally I've accidentally used the same variable for both and introduced a bug. So where one of those got in my way, I've renamed it to 'toret' or 'retd' (the latter short for 'returned') in line with my usual modern practice, but I haven't done a thorough job of finding all of them. Finally, one amusing side effect of doing this is that I've had to separate quite a few chained assignments. It used to be perfectly fine to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a the 'true' defined by stdbool.h, that idiom provokes a warning from gcc: 'suggest parentheses around assignment used as truth value'!
-
Simon Tatham authored
I think this is the full set of things that ought logically to be boolean. One annoyance is that quite a few radio-button controls in config.c address Conf fields that are now bool rather than int, which means that the shared handler function can't just access them all with conf_{get,set}_int. Rather than back out the rigorous separation of int and bool in conf.c itself, I've just added a similar alternative handler function for the bool-typed ones.
-
Simon Tatham authored
This commit includes <stdbool.h> from defs.h and deletes my traditional definitions of TRUE and FALSE, but other than that, it's a 100% mechanical search-and-replace transforming all uses of TRUE and FALSE into the C99-standardised lowercase spellings. No actual types are changed in this commit; that will come next. This is just getting the noise out of the way, so that subsequent commits can have a higher proportion of signal.
-
- Oct 11, 2018
-
-
Simon Tatham authored
This is a new vtable-based abstraction which is passed to a backend in place of Frontend, and it implements only the subset of the Frontend functions needed by a backend. (Many other Frontend functions still exist, notably the wide range of things called by terminal.c providing platform-independent operations on the GUI terminal window.) The purpose of making it a vtable is that this opens up the possibility of creating a backend as an internal implementation detail of some other activity, by providing just that one backend with a custom Seat that implements the methods differently. For example, this refactoring should make it feasible to directly implement an SSH proxy type, aka the 'jump host' feature supported by OpenSSH, aka 'open a secondary SSH session in MAINCHAN_DIRECT_TCP mode, and then expose the main channel of that as the Socket for the primary connection'. (Which of course you can already do by spawning 'plink -nc' as a separate proxy process, but this would permit it in the _same_ process without anything getting confused.) I've centralised a full set of stub methods in misc.c for the new abstraction, which allows me to get rid of several annoying stubs in the previous code. Also, while I'm here, I've moved a lot of duplicated modalfatalbox() type functions from application main program files into wincons.c / uxcons.c, which I think saves duplication overall. (A minor visible effect is that the prefixes on those console-based fatal error messages will now be more consistent between applications.)
-
Simon Tatham authored
This was used by ldisc to communicate back to the front end that a key had been pressed (or rather, that a keypress had caused a nonzero amount of session input data). Its only nontrivial implementation was in gtkwin.c, which used that notification to implement the Unix GUI's "close window on keypress, if the session was already over" policy. (Which in turn is Unix-specific, because the rationale is that sometimes X servers don't have a functioning window manager, so it's useful to have a way of telling any application to close without using WM-provided facilities like a close button.) But gtkwin.c doesn't need to be told by the ldisc that a keypress happened - it's the one _sending_ those keypresses to ldisc in the first place! So I've thrown away the three stub implementations of frontend_keypress, removed the call to it in ldisc.c, and replaced it with calls in gtkwin.c at all the points during keypress handling that call ldisc_send. A visible effect is that pterm's close-on-keypress behaviour will now only trigger on an actual (input-generating) _keypress_, and not on other input generation such as a paste action. I think that's an improvement.
-
- Sep 24, 2018
-
-
Simon Tatham authored
In order to list cross-certifiable host keys in the GUI specials menu, the SSH backend has been inventing new values on the end of the Telnet_Special enumeration, starting from the value TS_LOCALSTART. This is inelegant, and also makes it awkward to break up special handlers (e.g. to dispatch different specials to different SSH layers), since if all you know about a special is that it's somewhere in the TS_LOCALSTART+n space, you can't tell what _general kind_ of thing it is. Also, if I ever need another open-ended set of specials in future, I'll have to remember which TS_LOCALSTART+n codes are in which set. So here's a revamp that causes every special to take an extra integer argument. For all previously numbered specials, this argument is passed as zero and ignored, but there's a new main special code for SSH host key cross-certification, in which the integer argument is an index into the backend's list of available keys. TS_LOCALSTART is now a thing of the past: if I need any other open-ended sets of specials in future, I can add a new top-level code with a nicely separated space of arguments. While I'm at it, I've removed the legacy misnomer 'Telnet_Special' from the code completely; the enum is now SessionSpecialCode, the struct containing full details of a menu entry is SessionSpecial, and the enum values now start SS_ rather than TS_.
-
- Sep 19, 2018
-
-
Simon Tatham authored
This is another major source of unexplained 'void *' parameters throughout the code. In particular, the currently unused testback.c actually gave the wrong pointer type to its internal store of the frontend handle - it cast the input void * to a Terminal *, from which it got implicitly cast back again when calling from_backend, and nobody noticed. Now it uses the right type internally as well as externally.
-
Simon Tatham authored
Nearly every part of the code that ever handles a full backend structure has historically done it using a pair of pointer variables, one pointing at a constant struct full of function pointers, and the other pointing to a 'void *' state object that's passed to each of those. While I'm modernising the rest of the code, this seems like a good time to turn that into the same more or less type-safe and less cumbersome system as I'm using for other parts of the code, such as Socket, Plug, BinaryPacketProtocol and so forth: the Backend structure contains a vtable pointer, and a system of macro wrappers handles dispatching through that vtable.
-
Simon Tatham authored
That's one fewer anonymous 'void *' which might be accidentally confused with some other pointer type if I misremember the order of function arguments. While I'm here, I've made its pointer-nature explicit - that is, 'Ldisc' is now a typedef for the structure type itself rather than a pointer to it. A stylistic change only, but it feels more natural to me these days for a thing you're going to eventually pass to a 'free' function.
-
- May 26, 2018
-
-
Simon Tatham authored
This is a cleanup I started to notice a need for during the BinarySink work. It removes a lot of faffing about casting things to char * or unsigned char * so that some API will accept them, even though lots of such APIs really take a plain 'block of raw binary data' argument and don't care what C thinks the signedness of that data might be - they may well reinterpret it back and forth internally. So I've tried to arrange for all the function call APIs that ought to have a void * (or const void *) to have one, and those that need to do pointer arithmetic on the parameter internally can cast it back at the top of the function. That saves endless ad-hoc casts at the call sites.
-
- May 15, 2015
-
-
Simon Tatham authored
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
-
- Nov 22, 2014
-
-
Simon Tatham authored
I'm not actually sure why we've always had back ends notify ldisc of changes to echo/edit settings by giving ldisc_send(ldisc,NULL,0,0) a special meaning, instead of by having a separate dedicated notify function with its own prototype and parameter set. Coverity's recent observation that the two kinds of call don't even have the same requirements on the ldisc (particularly, whether ldisc->term can be NULL) makes me realise that it's really high time I separated the two conceptually different operations into actually different functions. While I'm here, I've renamed the confusing ldisc_update() function which that special operation ends up feeding to, because it's not actually a function applying to an ldisc - it applies to a front end. So ldisc_send(ldisc,NULL,0,0) is now ldisc_echoedit_update(ldisc), and that in turn figures out the current echo/edit settings before passing them on to frontend_echoedit_update(). I think that should be clearer.
-
Simon Tatham authored
Namely, any ldisc that you send actual data through should have a terminal attached, because the ldisc editing/echoing system is designed entirely for use with a terminal. The only time you can have an ldisc with no terminal is when it's only ever used by the backend to report changes to the front end in edit/echo status, e.g. by Unix Plink. Coverity spotted an oddity in ldisc_send which after a while I decided would never have actually caused a problem, but OTOH I agree that it was confusing, so now hopefully it's less so.
-
- Aug 17, 2013
-
-
Simon Tatham authored
It was one of those things that went in ages ago on Windows and never got replicated in the Unix front end. And it needn't be: ldisc.c is a perfect place to put it, since it knows which of the data it's sending is based on a keystroke and which is automatically generated, and it also has access to the terminal context. So now a keypress can interrupt a runaway paste on all platforms. [originally from svn r10025]
-
- Jul 14, 2011
-
-
Simon Tatham authored
'Config' in putty.h, which stores all PuTTY's settings and includes an arbitrary length limit on every single one of those settings which is stored in string form. In place of it is 'Conf', an opaque data type everywhere outside the new file conf.c, which stores a list of (key, value) pairs in which every key contains an integer identifying a configuration setting, and for some of those integers the key also contains extra parts (so that, for instance, CONF_environmt is a string-to-string mapping). Everywhere that a Config was previously used, a Conf is now; everywhere there was a Config structure copy, conf_copy() is called; every lookup, adjustment, load and save operation on a Config has been rewritten; and there's a mechanism for serialising a Conf into a binary blob and back for use with Duplicate Session. User-visible effects of this change _should_ be minimal, though I don't doubt I've introduced one or two bugs here and there which will eventually be found. The _intended_ visible effects of this change are that all arbitrary limits on configuration strings and lists (e.g. limit on number of port forwardings) should now disappear; that list boxes in the configuration will now be displayed in a sorted order rather than the arbitrary order in which they were added to the list (since the underlying data structure is now a sorted tree234 rather than an ad-hoc comma-separated string); and one more specific change, which is that local and dynamic port forwardings on the same port number are now mutually exclusive in the configuration (putting 'D' in the key rather than the value was a mistake in the first place). One other reorganisation as a result of this is that I've moved all the dialog.c standard handlers (dlg_stdeditbox_handler and friends) out into config.c, because I can't really justify calling them generic any more. When they took a pointer to an arbitrary structure type and the offset of a field within that structure, they were independent of whether that structure was a Config or something completely different, but now they really do expect to talk to a Conf, which can _only_ be used for PuTTY configuration, so I've renamed them all things like conf_editbox_handler and moved them out of the nominally independent dialog-box management module into the PuTTY-specific config.c. [originally from svn r9214]
-
- Sep 09, 2010
-
-
Simon Tatham authored
extension. Since ldisc_send() uses bit 8 as an internal flag, we shouldn't be setting it except when we really want to. [originally from svn r8989]
-
- May 22, 2004
-
-
Simon Tatham authored
enhancement and fiddling, I have now massaged Arabeyes' first patch into a form I'm happy to check in. Phew. [originally from svn r4236]
-
- Apr 12, 2003
-
-
Simon Tatham authored
from_backend(), and must now pass its frontend handle rather than its terminal handle. [originally from svn r3106]
-
- Mar 29, 2003
-
-
Simon Tatham authored
malloc functions, which automatically cast to the same type they're allocating the size of. Should prevent any future errors involving mallocing the size of the wrong structure type, and will also make life easier if we ever need to turn the PuTTY core code from real C into C++-friendly C. I haven't touched the Mac frontend in this checkin because I couldn't compile or test it. [originally from svn r3014]
-
- Mar 11, 2003
-
-
Simon Tatham authored
letting me know about instances of this, but it turns out that my ctype.h explicitly casts input values to `int' to evade the `subscript has type char' warning, so it had been carefully not letting me know! Found them all by compiling with a doctored ctype.h, and hopefully fixed them all too. [originally from svn r2927]
-
- Jan 27, 2003
-
-
Simon Tatham authored
Everything in there which is integral is now an actual int, which means my forthcoming revamp of the config box will be able to work with `int *' pointers without fear of doom. [originally from svn r2733]
-
- Jan 16, 2003
-
-
Ben Harris authored
areas of the code. Not all back-ends have been tested, but Telnet and SSH behave reasonably. Incidentally, almost all of this patch was written through Mac PuTTY, admittedly over a Telnet connection. [originally from svn r2615]
-
- Nov 23, 2002
-
-
Ben Harris authored
cfg throughout ldisc.c. Not tested other than on Mac, but all other ports just pass &cfg as this argument for now. [originally from svn r2250]
-
- Oct 26, 2002
-
-
Simon Tatham authored
lpage_send out into the line discipline, making them _clients_ of the Unicode layer rather than part of it. This means they can access ldisc->term, which in turn means I've been able to remove the temporary global variable `term'. We're slowly getting there. [originally from svn r2143]
-
Simon Tatham authored
fixed one or two other minor problems. [originally from svn r2141]
-
- Oct 25, 2002
-
-
Simon Tatham authored
each backend now stores all its internal variables in a big struct, and each backend function gets a pointer to this struct passed to it. This still isn't the end of the work - lots of subsidiary things still use globals, notably all the cipher and compressor modules and the X11 forwarding authentication stuff. But ssh.c itself has now been transformed, and that was the really painful bit, so from here on it all ought to be a sequence of much smaller and simpler pieces of work. [originally from svn r2127]
-
- Oct 24, 2002
-
-
Simon Tatham authored
check in. I must stop doing my Unix checkins in the Unix subdir :-( [originally from svn r2125]
-
- Oct 22, 2002
-
-
Simon Tatham authored
all the global and function-static variables out of terminal.c into a dynamically allocated data structure. Note that this does not yet confer the ability to run more than one of them in the same process, because other things (the line discipline, the back end) are still global, and also in particular the address of the dynamically allocated terminal-data structure is held in a global variable `term'. But what I've got here represents a reasonable stopping point at which to check things in. In _theory_ this should all still work happily, on both Unix and Windows. In practice, who knows? [originally from svn r2115]
-
- Oct 09, 2002
-
-
Simon Tatham authored
The current pty.c backend is temporarily a loopback device for terminal emulator testing, the display handling is only just enough to show that terminal.c is functioning, the keyboard handling is laughable, and most features are absent. Next step: bring output and input up to a plausibly working state, and put a real pty on the back to create a vaguely usable prototype. Oh, and a scrollbar would be nice too. In _theory_ the Windows builds should still work fine after this... [originally from svn r2010]
-
- Dec 29, 2001
-
-
Simon Tatham authored
^M instead of the Telnet New Line code. Unix-type telnetds don't care one way or the other; RDB claims some telnetds prefer Telnet NL; and now someone has found one that can't deal with Telnet NL and prefers ^M. Sigh. [originally from svn r1520]
-
- Oct 24, 2001
-
-
Simon Tatham authored
and friends, we should honour the user's choice even in line editing mode. In particular, Telnet talkers don't like us randomly spraying Telnet IP whenever the user accidentally hits ^C, so this is not a helpful default. [originally from svn r1316]
-
- Sep 19, 2001
-
-
Simon Tatham authored
very _good_ fix; something might want doing after the release. [originally from svn r1277]
-
- May 19, 2001
-
-
Simon Tatham authored
[originally from svn r1138]
-
- May 10, 2001
-
-
Simon Tatham authored
possible and we have a single unified means of trying to display any Unicode code point. Instead of the various ad-hoc translation modes we had before, we now have a single `codepage' option which allows us to treat the incoming (and outgoing) text as any given character set, and locally we map that to Unicode and back. [originally from svn r1110]
-
- May 09, 2001
-
-
Simon Tatham authored
send Telnet special sequences (Interrupt Process, Suspend, Erase Char, End Of Line) instead of their ASCII equivalents. In particular Return -> Telnet End Of Line is _always_ enabled irrespective of the configuration, while the others are optional. Also in this patch, an entertainingly ghastly use of `switch' to allow literal ^M^J to do the same thing as magic-^M (the Return key) when in Raw protocol. [originally from svn r1109]
-