Skip to content
Snippets Groups Projects
  1. Feb 16, 2020
    • Simon Tatham's avatar
      Formatting change to braces around one case of a switch. · 8d186c3c
      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.
      8d186c3c
  2. Jan 29, 2020
    • Simon Tatham's avatar
      Add lots of missing 'static' keywords. · 8d747d80
      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.)
      8d747d80
  3. Sep 08, 2019
    • Simon Tatham's avatar
      Whitespace rationalisation of entire code base. · 5d718ef6
      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.
      5d718ef6
  4. Mar 16, 2019
    • Simon Tatham's avatar
      Add a per-line 'trusted' status in Terminal. · 9c367eba
      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.
      9c367eba
  5. Feb 25, 2019
  6. Feb 09, 2019
    • Simon Tatham's avatar
      minibidi: fix read past end of line in rule W5. · 03492ab5
      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
      03492ab5
  7. Nov 03, 2018
    • Simon Tatham's avatar
      Add missing 'static' on file-internal declarations. · 91d16881
      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.
      91d16881
    • Simon Tatham's avatar
      Convert a lot of 'int' variables to 'bool'. · 3214563d
      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'!
      3214563d
  8. Jun 20, 2017
    • Simon Tatham's avatar
      Fix a collection of type / format string mismatches. · 20e36ae4
      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.
      20e36ae4
  9. Apr 15, 2017
  10. Sep 24, 2014
    • Simon Tatham's avatar
      Rework versioning system to not depend on Subversion. · 4d8782e7
      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]
      4d8782e7
  11. Mar 05, 2012
  12. Feb 17, 2012
    • Simon Tatham's avatar
      Patch from Yoshida Masato to fill in the missing pieces of Windows · 053d2ba6
      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]
      053d2ba6
  13. May 07, 2011
  14. Jun 29, 2008
  15. Nov 18, 2006
    • Simon Tatham's avatar
      Reinstate as much of the Windows font-linking behaviour as I can · 230d400d
      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]
      230d400d
  16. Jan 16, 2005
  17. Dec 20, 2004
    • Simon Tatham's avatar
      The end condition in the binary search loop in the new getType() was · e202f0f2
      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]
      e202f0f2
  18. Dec 08, 2004
    • Simon Tatham's avatar
      Replace the RLE-based getType() function with one that binary- · e7111211
      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]
      e7111211
    • Simon Tatham's avatar
      Further clarity and speed cleanups of minibidi: · 45534b2a
      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]
      45534b2a
  19. Dec 07, 2004
  20. Nov 29, 2004
  21. Nov 28, 2004
  22. May 22, 2004
Loading