Skip to content
Snippets Groups Projects
  1. 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
  2. Nov 03, 2018
    • 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
    • Simon Tatham's avatar
      Adopt C99 <stdbool.h>'s true/false. · a6f1709c
      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.
      a6f1709c
  3. Oct 04, 2018
    • Simon Tatham's avatar
      Get rid of lots of implicit pointer types. · 96ec2c25
      Simon Tatham authored
      All the main backend structures - Ssh, Telnet, Pty, Serial etc - now
      describe structure types themselves rather than pointers to them. The
      same goes for the codebase-wide trait types Socket and Plug, and the
      supporting types SockAddr and Pinger.
      
      All those things that were typedefed as pointers are older types; the
      newer ones have the explicit * at the point of use, because that's
      what I now seem to be preferring. But whichever one of those is
      better, inconsistently using a mixture of the two styles is worse, so
      let's make everything consistent.
      
      A few types are still implicitly pointers, such as Bignum and some of
      the GSSAPI types; generally this is either because they have to be
      void *, or because they're typedefed differently on different
      platforms and aren't always pointers at all. Can't be helped. But I've
      got rid of the main ones, at least.
      96ec2c25
  4. Sep 24, 2018
    • Simon Tatham's avatar
      Rework special-commands system to add an integer argument. · f4fbaa1b
      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_.
      f4fbaa1b
  5. Sep 19, 2018
    • Simon Tatham's avatar
      Turn Backend into a sensible classoid. · eefebaaa
      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.
      eefebaaa
  6. Apr 02, 2016
    • Simon Tatham's avatar
      Fix a potential time-wraparound issue in pinger.c. · b4202c91
      Simon Tatham authored
      A compiler warning drew my attention to the fact that 'next' in
      pinger_schedule() was an int, not the unsigned long it should have
      been. And looking at the code that handles it, it was also taking no
      care with integer wraparound when checking whether an existing
      scheduled ping should be moved forward.
      
      So now I do something a bit more robust, by remembering what time it
      _was_ when we set pinger->next, and checking if the new time value
      falls in the interval between those two times.
      b4202c91
  7. Sep 18, 2012
    • Ben Harris's avatar
      Two related changes to timing code: · d5836982
      Ben Harris authored
      First, make absolute times unsigned.  This means that it's safe to 
      depend on their overflow behaviour (which is undefined for signed 
      integers).  This requires a little extra care in handling comparisons, 
      but I think I've correctly adjusted them all.
      
      Second, functions registered with schedule_timer() are guaranteed to be 
      called with precisely the time that was returned by schedule_timer().  
      Thus, it's only necessary to check these values for equality rather than 
      doing risky range checks, so do that.
      
      The timing code still does lots that's undefined, unnecessary, or just
      wrong, but this is a good start.
      
      [originally from svn r9667]
      d5836982
  8. Jul 14, 2011
    • Simon Tatham's avatar
      Post-release destabilisation! Completely remove the struct type · a1f3b7a3
      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]
      a1f3b7a3
  9. Nov 27, 2004
    • Simon Tatham's avatar
      New timing infrastructure. There's a new function schedule_timer() · 7ecf1356
      Simon Tatham authored
      which pretty much any module can call to request a call-back in the
      future. So terminal.c can do its own handling of blinking, visual
      bells and deferred screen updates, without having to rely on
      term_update() being called 50 times a second (fixes: pterm-timer);
      and ssh.c and telnet.c both invoke a new module pinger.c which takes
      care of sending keepalives, so they get sent uniformly in all front
      ends (fixes: plink-keepalives, unix-keepalives).
      
      [originally from svn r4906]
      [this svn revision also touched putty-wishlist]
      7ecf1356
Loading