simon-git: putty (master): Simon Tatham

[Commits to Tartarus hosted VCS]

TL;DR:
  bd5c957e winsftp.c: avoid creating multiple netevents.
  421a8ca5 Fix cursor save/restore with [?1047 alt-screen sequences.
  5e468129 Refactor 'struct context *ctx = &actx' pattern.

Repository:     https://git.tartarus.org/simon/putty.git
On the web:     https://git.tartarus.org/?p=simon/putty.git
Branch updated: master
Committer:      Simon Tatham <anakin at pobox.com>
Date:           2019-12-25 22:46:40

commit bd5c957e5bd0f850ab30c6b1ab94bfbbabe9f006
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=bd5c957e5bd0f850ab30c6b1ab94bfbbabe9f006;hp=83408f928de23f033db211eb107825631bbcb390
Author: Simon Tatham <anakin at pobox.com>
Date:   Sat Dec 21 13:31:02 2019 +0000

    winsftp.c: avoid creating multiple netevents.
    
    The do_select function is called with a boolean parameter indicating
    whether we're supposed to start or stop paying attention to network
    activity on a given socket. So if we freeze and unfreeze the socket in
    mid-session because of backlog, we'll call do_select(s, false) to
    freeze it, and do_select(s, true) to unfreeze it.
    
    But the implementation of do_select in the Windows SFTP code predated
    the rigorous handling of socket backlogs, so it assumed that
    do_select(s, true) would only be called at initialisation time, i.e.
    only once, and therefore that it was safe to use that flag as a cue to
    set up the Windows event object to associate with socket activity.
    Hence, every time the socket was frozen and unfrozen, we would create
    a new netevent at unfreeze time, leaking the old one.
    
    I think perhaps part of the reason why that was hard to figure out was
    that the boolean parameter was called 'startup' rather than 'enable'.
    To make it less confusing the next time I read this code, I've also
    renamed it, and while I was at it, adjusted another related comment.

 windows/window.c   |  4 ++--
 windows/winplink.c |  4 ++--
 windows/winsftp.c  | 14 +++++++++-----
 windows/winstuff.h |  2 +-
 4 files changed, 14 insertions(+), 10 deletions(-)

commit 421a8ca5d9e78bc1ac0df0c6dacc04756f5a96e0
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=421a8ca5d9e78bc1ac0df0c6dacc04756f5a96e0;hp=bd5c957e5bd0f850ab30c6b1ab94bfbbabe9f006
Author: Simon Tatham <anakin at pobox.com>
Date:   Tue Dec 24 10:52:38 2019 +0000

    Fix cursor save/restore with [?1047 alt-screen sequences.
    
    A long time ago, in commit 09f86ce7e, I introduced a separate copy of
    the saved cursor position (used by the ESC 7 / ESC 8 sequences) for
    the main and alternate screens. The idea was to fix mishandling of an
    input sequence of the form
    
      ESC 7        (save cursor)
      ESC [?47h    (switch to alternate screen)
      ...
      ESC 7 ESC 8  (save and restore cursor, while in alternate screen)
      ...
      ESC [?47l    (switch back from alternate screen)
      ESC 8        (restore cursor, expecting it to match the _first_ ESC 7)
    
    in which, before the fix, the second ESC 7 would overwrite the
    position saved by the first one. So the final ESC 8 would restore the
    cursor position to wherever it happened to have been saved in the
    alternate screen, instead of where it was saved before switching _to_
    the alternate screen.
    
    I've recently noticed that the same bug still happens if you use the
    alternative escape sequences ESC[?1047h and ESC[?1047l to switch to
    the alternate screen, instead of ESC[?47h and ESC[?47l. This is
    because that version of the escape sequence sets the internal flag
    'keep_cur_pos' in the call to swap_screen, whose job is to arrange
    that the actual cursor position doesn't change at the instant of the
    switch. But the code that swaps the _saved_ cursor position in and out
    is also conditioned on keep_cur_pos, so the 1047 variant of the
    screen-swap sequence was bypassing that too, and behaving as if there
    was just a single saved cursor position inside and outside the
    alternate screen.
    
    I don't know why I did it that way in 2006. It could have been
    deliberate for some reason, or it could just have been mindless copy
    and paste from the existing cursor-related swap code. But checking
    with xterm now, it definitely seems to be wrong: the 1047 screen swap
    preserves the _actual_ cursor position across the swap, but still has
    independent _saved_ cursor positions in the two screens. So now PuTTY
    does the same.

 terminal.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

commit 5e468129f6f82bd06f3b19fac1885750bb8b0c91
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=5e468129f6f82bd06f3b19fac1885750bb8b0c91;hp=421a8ca5d9e78bc1ac0df0c6dacc04756f5a96e0
Author: Simon Tatham <anakin at pobox.com>
Date:   Sun Dec 22 08:15:52 2019 +0000

    Refactor 'struct context *ctx = &actx' pattern.
    
    When I'm declaring a local instance of some context structure type to
    pass to a function which will pass it in turn to a callback, I've
    tended to use a declaration of the form
    
        struct context actx, *ctx = &actx;
    
    so that the outermost caller can initialise the context, and/or read
    out fields of it afterwards, by the same syntax 'ctx->foo' that the
    callback function will be using. So you get visual consistency between
    the two functions that share this context.
    
    It only just occurred to me that there's a much neater way to declare
    a context struct of this kind, which still makes 'ctx' behave like a
    pointer in the owning function, and doesn't need all that weird
    verbiage or a spare variable name:
    
        struct context ctx[1];
    
    That's much nicer! I've switched to doing that in all existing cases I
    could find, and also in a couple of cases where I hadn't previously
    bothered to do the previous more cumbersome idiom.

 psftp.c           |  4 ++--
 ssh.c             | 14 +++++++-------
 tree234.c         | 16 ++++++++--------
 unix/gtkask.c     |  2 +-
 unix/gtkwin.c     |  2 +-
 unix/uxpgnt.c     |  2 +-
 windows/winsftp.c |  2 +-
 7 files changed, 21 insertions(+), 21 deletions(-)