- Feb 16, 2020
-
-
Simon Tatham authored
Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
-
- Jan 29, 2020
-
-
Simon Tatham authored
A trawl through the code with -Wmissing-prototypes and -Wmissing-variable-declarations turned up a lot of things that should have been internal to a particular source file, but were accidentally global. Keep the namespace clean by making them all static. (Also, while I'm here, a couple of them were missing a 'const': the ONE and ZERO arrays in sshcrcda.c, and EMPTY_WINDOW_TITLE in terminal.c.)
-
- 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.
-
- Mar 16, 2019
-
-
Simon Tatham authored
This indicates that a line contains trusted information (originated by PuTTY) or untrusted (from the server). Trusted lines are prefixed by a three-column signature consisting of the trust sigil (i.e. PuTTY icon) and a separating space. To protect against a server using escape sequences to move the cursor back up to a trusted line and overwrite its contents, any attempt to write to a termline is preceded by a call to check_trust_status(), which clears the line completely if the terminal's current trust status is different from the previous state of that line. In the terminal data structures, the trust sigil is represented by 0xDFFE (an otherwise unused value, because it's in the surrogate space). For bidi purposes I've arranged to treat that value as direction-neutral, so that it will appear on the right if a terminal line needs it to. (Not that that's currently likely to happen, with PuTTY not being properly localised, but it's a bit of futureproofing.) The bidi system is also where I actually insert the trust sigil: the _logical_ terminal data structures don't include it. term_bidi_line was a convenient place to add it, because that function was already transforming a logical terminal line into a physical one in a way that also generates a logical<->physical mapping table for handling mouse clicks and cursor positioning; so that function now adds the trust sigil as well as running the bidi algorithm. (A knock-on effect of _that_ is that the log<->phys position map now has to have a value for 'no correspondence', because if the user does click on the trust sigil, there's no logical terminal position corresponding to that. So the map can now contain the special value BIDI_CHAR_INDEX_NONE, and anyone looking things up in it has to be prepared to receive that as an answer.) Of course, this terminal-data transformation can't be kept _wholly_ within term_bidi_line, because unlike proper bidi, it actually reduces the number of visible columns on the line. So the wrapping code (during glyph display and also copy and paste) has to take account of the trusted status and use it to ignore the last 3 columns of the line. This is probably not done absolutely perfectly, but then, it doesn't need to be - trusted lines will be filled with well-controlled data generated from the SSH code, which won't be doing every trick in the book with escape sequences. Only untrusted terminal lines will be using all the terminal's capabilities, and they don't have this sigil getting in the way.
-
- Feb 25, 2019
-
-
Simon Tatham authored
The bidi_char structure was declared twice, with nothing to keep them in sync; also, is_rtl had a mismatch of return types.
-
- Feb 09, 2019
-
-
Simon Tatham authored
The check for a sequence of ET with an EN after it could potentially skip ETs all the way up to the end of the buffer and then look for an EN in the following nonexistent array element. Now it only skips ETs up to count-1, in the same style as the similar check in rule N1. Change-Id: Ifdbae494a22d1b96bf49ae1bcae0efb901565f45
-
- Nov 03, 2018
-
-
Simon Tatham authored
sk_startup and sk_nextaddr are entirely internal to winnet.c; nearly all of import.c and minibidi.c's internal routines should have been static and weren't; {read,write}_utf8 are internal to charset/utf8.c (and didn't even need separate declarations at all); do_sftp_cleanup is internal to psftp.c, and get_listitemheight to gtkdlg.c. While I was editing those prototypes anyway, I've also added missing 'const' to the 'char *' passphrase parameters in import,c.
-
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'!
-
- Jun 20, 2017
-
-
Simon Tatham authored
Ilya Shipitsin sent me a list of errors reported by a tool 'cppcheck', which I hadn't seen before, together with some fixes for things already taken off that list. This change picks out all the things from the remaining list that I could quickly identify as actual errors, which it turns out are all format-string goofs along the lines of using a %d with an unsigned int, or a %u with a signed int, or (in the cases in charset/utf8.c) an actual _size_ mismatch which could in principle have caused trouble on a big-endian target.
-
- Apr 15, 2017
-
-
klemens authored
As found by a bot ( http://www.misfix.org, https://github.com/ka7/misspell_fixer ).
-
- Sep 24, 2014
-
-
Simon Tatham authored
I've shifted away from using the SVN revision number as a monotonic version identifier (replacing it in the Windows version resource with a count of days since an arbitrary epoch), and I've removed all uses of SVN keyword expansion (replacing them with version information written out by Buildscr). While I'm at it, I've done a major rewrite of the affected code which centralises all the computation of the assorted version numbers and strings into Buildscr, so that they're all more or less alongside each other rather than scattered across multiple source files. I've also retired the MD5-based manifest file system. A long time ago, it seemed like a good idea to arrange that binaries of PuTTY would automatically cease to identify themselves as a particular upstream version number if any changes were made to the source code, so that if someone made a local tweak and distributed the result then I wouldn't get blamed for the results. Since then I've decided the whole idea is more trouble than it's worth, so now distribution tarballs will have version information baked in and people can just cope with that. [originally from svn r10262]
-
- Mar 05, 2012
-
-
Simon Tatham authored
bidi_char from wchar_t to unsigned int, but omitted to similarly adjust the parameter to doMirror which is passed a pointer to that field. [originally from svn r9426] [r9409 == 053d2ba6]
-
- Feb 17, 2012
-
-
Simon Tatham authored
UTF-16 support. High Unicode characters in the terminal are now converted back into surrogates during copy and draw operations, and the Windows drawing code takes account of that when splitting up the UTF-16 string for display. Meanwhile, accidental uses of wchar_t have been replaced with 32-bit integers in parts of the cross-platform code which were expecting not to have to deal with UTF-16. [originally from svn r9409]
-
- May 07, 2011
-
-
Simon Tatham authored
PuTTY compile cleanly under gcc 4.6.0 without triggering any of its new warnings. [originally from svn r9169]
-
- Jun 29, 2008
-
-
Simon Tatham authored
Persian, by adding some additional Unicode code points to the shapetypes[] array. [originally from svn r8097]
-
- Nov 18, 2006
-
-
Simon Tatham authored
easily manage, by adopting a hybrid approach to Unicode text display. The old approach of simply calling ExtTextOutW provided font linking without us having to lift a finger, but didn't do the right thing when it came to bidirectional or Arabic-shaped text. Arabeyes' replacement exact_textout() supported the latter, but turned out to break the former (with no warning from the Windows API documentation, so it's not their fault). So now I've got a second wrapper layer called general_textout(), which splits the input string into substrings based on bidi character class. Any character liable to cause bidi or shaping behaviour if fed straight to ExtTextOutW is instead fed through Arabeyes' exact_textout(), but the rest is fed straight to ExtTextOutW as it used to be. The effect appears to be that font linking is restored for all characters _except_ Arabic and other bidi scripts, which means in particular that we are no longer in a state of regression over 0.57. (0.57 would have done font linking on Arabic as well, but would also have misbidied it, so we've merely exchanged one failure mode for another slightly less harmful one in that situation.) [originally from svn r6910]
-
- Jan 16, 2005
-
-
Owen Dunn authored
[originally from svn r5120]
-
- Dec 20, 2004
-
-
Simon Tatham authored
incorrect. I must have written that binary search idiom a hundred times, so it's rather embarrassing that I can't _automatically_ get it right! This was causing all kinds of characters to be classified as ON when they should have been various other classes. Also while I'm here, I've added another test case to utf8.txt (a small piece of Arabic within a predominantly L->R line), and also supplied a means to compile minibidi.c with -DTEST_GETTYPE to produce a command-line character class lookup tool. (Not sure what use that'll be _other_ than debugging this precise problem, but I don't like to throw it away now I've written it :-) [originally from svn r5016]
-
- Dec 08, 2004
-
-
Simon Tatham authored
searches a list of (start,end,type) tuples. This increases data size by about 5Kb, which is a shame; but on the plus side, it boosts performance from O(N) to O(log N). As an added bonus, the table now covers _all_ of Unicode, not just the BMP. [originally from svn r4964]
-
Simon Tatham authored
- rewrote the reversal loop in flipThisRun to be considerably clearer - rewrote leastGreaterOdd and leastGreaterEven as bit-twiddling macros - replaced malloc/free with snewn/sfree - lost some gratuitous repeat calls of getType on the same character And most noticeably: - got rid of minibidi.h, since it was entirely full of minibidi.c internals (including constant data definitions!) and wasn't used to provide an external interface at all. Everything in it has been folded into minibidi.c. [originally from svn r4963]
-
- Dec 07, 2004
-
-
Simon Tatham authored
just getting to be too much hassle trying to work with the existing indentation. [originally from svn r4952]
-
Simon Tatham authored
array bounds checking. [originally from svn r4951]
-
- Nov 29, 2004
-
-
Jacob Nevins authored
[originally from svn r4924]
-
- Nov 28, 2004
-
-
Simon Tatham authored
[originally from svn r4916]
-
- 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]
-