From c269dd0135a927d4d22a334cfefb09361f311fcd Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 22 Nov 2014 16:12:47 +0000 Subject: [PATCH 1/1] Move echo/edit state change functionality out of ldisc_send. 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. --- ldisc.c | 23 +++++++---------------- macosx/osxwin.m | 2 +- pscp.c | 11 +---------- psftp.c | 11 +---------- putty.h | 3 ++- ssh.c | 4 ++-- telnet.c | 2 +- terminal.c | 8 ++++---- unix/gtkwin.c | 8 ++++---- unix/uxplink.c | 9 +++++---- windows/window.c | 6 +++--- windows/winplink.c | 2 +- 12 files changed, 32 insertions(+), 57 deletions(-) diff --git a/ldisc.c b/ldisc.c index 31185406..0e864874 100644 --- a/ldisc.c +++ b/ldisc.c @@ -128,28 +128,19 @@ void ldisc_free(void *handle) sfree(ldisc); } +void ldisc_echoedit_update(void *handle) +{ + Ldisc ldisc = (Ldisc) handle; + frontend_echoedit_update(ldisc->frontend, ECHOING, EDITING); +} + void ldisc_send(void *handle, char *buf, int len, int interactive) { Ldisc ldisc = (Ldisc) handle; int keyflag = 0; - /* - * Called with len=0 when the options change. We must inform - * the front end in case it needs to know. - */ - if (len == 0) { - ldisc_update(ldisc->frontend, ECHOING, EDITING); - return; - } - /* - * If that wasn't true, then we expect ldisc->term to be non-NULL - * hereafter. (The only front ends which have an ldisc but no term - * are those which do networking but no terminal emulation, in - * which case they need the above if statement to handle - * ldisc_updates passed from the back ends, but should never send - * any actual input through this function.) - */ assert(ldisc->term); + assert(len); /* * Notify the front end that something was pressed, in case diff --git a/macosx/osxwin.m b/macosx/osxwin.m index b58742d9..3bf85cd8 100644 --- a/macosx/osxwin.m +++ b/macosx/osxwin.m @@ -907,7 +907,7 @@ void notify_remote_exit(void *frontend) [win notifyRemoteExit]; } -void ldisc_update(void *frontend, int echo, int edit) +void frontend_echoedit_update(void *frontend, int echo, int edit) { //SessionWindow *win = (SessionWindow *)frontend; /* diff --git a/pscp.c b/pscp.c index e56d760f..50ab90b5 100644 --- a/pscp.c +++ b/pscp.c @@ -60,16 +60,7 @@ const char *const appname = "PSCP"; */ #define MAX_SCP_BUFSIZE 16384 -void ldisc_send(void *handle, char *buf, int len, int interactive) -{ - /* - * This is only here because of the calls to ldisc_send(NULL, - * 0) in ssh.c. Nothing in PSCP actually needs to use the ldisc - * as an ldisc. So if we get called with any real data, I want - * to know about it. - */ - assert(len == 0); -} +void ldisc_echoedit_update(void *handle) { } static void tell_char(FILE * stream, char c) { diff --git a/psftp.c b/psftp.c index 3fbd0c43..3543a06e 100644 --- a/psftp.c +++ b/psftp.c @@ -2506,16 +2506,7 @@ void connection_fatal(void *frontend, char *fmt, ...) cleanup_exit(1); } -void ldisc_send(void *handle, char *buf, int len, int interactive) -{ - /* - * This is only here because of the calls to ldisc_send(NULL, - * 0) in ssh.c. Nothing in PSFTP actually needs to use the - * ldisc as an ldisc. So if we get called with any real data, I - * want to know about it. - */ - assert(len == 0); -} +void ldisc_echoedit_update(void *handle) { } /* * In psftp, all agent requests should be synchronous, so this is a diff --git a/putty.h b/putty.h index d8bddbae..520de490 100644 --- a/putty.h +++ b/putty.h @@ -602,7 +602,7 @@ void begin_session(void *frontend); void sys_cursor(void *frontend, int x, int y); void request_paste(void *frontend); void frontend_keypress(void *frontend); -void ldisc_update(void *frontend, int echo, int edit); +void frontend_echoedit_update(void *frontend, int echo, int edit); /* It's the backend's responsibility to invoke this at the start of a * connection, if necessary; it can also invoke it later if the set of * special commands changes. It does not need to invoke it at session @@ -1067,6 +1067,7 @@ void *ldisc_create(Conf *, Terminal *, Backend *, void *, void *); void ldisc_configure(void *, Conf *); void ldisc_free(void *); void ldisc_send(void *handle, char *buf, int len, int interactive); +void ldisc_echoedit_update(void *handle); /* * Exports from ldiscucs.c. diff --git a/ssh.c b/ssh.c index a69bca57..c4c4fb90 100644 --- a/ssh.c +++ b/ssh.c @@ -5832,7 +5832,7 @@ static void do_ssh1_connection(Ssh ssh, unsigned char *in, int inlen, ssh_special(ssh, TS_EOF); if (ssh->ldisc) - ldisc_send(ssh->ldisc, NULL, 0, 0);/* cause ldisc to notice changes */ + ldisc_echoedit_update(ssh->ldisc); /* cause ldisc to notice changes */ ssh->send_ok = 1; ssh->channels = newtree234(ssh_channelcmp); while (1) { @@ -10326,7 +10326,7 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen, * Transfer data! */ if (ssh->ldisc) - ldisc_send(ssh->ldisc, NULL, 0, 0);/* cause ldisc to notice changes */ + ldisc_echoedit_update(ssh->ldisc); /* cause ldisc to notice changes */ if (ssh->mainchan) ssh->send_ok = 1; while (1) { diff --git a/telnet.c b/telnet.c index 098db292..8717a972 100644 --- a/telnet.c +++ b/telnet.c @@ -264,7 +264,7 @@ static void option_side_effects(Telnet telnet, const struct Opt *o, int enabled) else if (o->option == TELOPT_SGA && o->send == DO) telnet->editing = !enabled; if (telnet->ldisc) /* cause ldisc to notice the change */ - ldisc_send(telnet->ldisc, NULL, 0, 0); + ldisc_echoedit_update(telnet->ldisc); /* Ensure we get the minimum options */ if (!telnet->activated) { diff --git a/terminal.c b/terminal.c index ac8f5dff..7570a63b 100644 --- a/terminal.c +++ b/terminal.c @@ -1350,7 +1350,7 @@ void term_pwron(Terminal *term, int clear) { power_on(term, clear); if (term->ldisc) /* cause ldisc to notice changes */ - ldisc_send(term->ldisc, NULL, 0, 0); + ldisc_echoedit_update(term->ldisc); term->disptop = 0; deselect(term); term_update(term); @@ -2574,7 +2574,7 @@ static void toggle_mode(Terminal *term, int mode, int query, int state) case 10: /* DECEDM: set local edit mode */ term->term_editing = state; if (term->ldisc) /* cause ldisc to notice changes */ - ldisc_send(term->ldisc, NULL, 0, 0); + ldisc_echoedit_update(term->ldisc); break; case 25: /* DECTCEM: enable/disable cursor */ compatibility2(OTHER, VT220); @@ -2638,7 +2638,7 @@ static void toggle_mode(Terminal *term, int mode, int query, int state) case 12: /* SRM: set echo mode */ term->term_echoing = !state; if (term->ldisc) /* cause ldisc to notice changes */ - ldisc_send(term->ldisc, NULL, 0, 0); + ldisc_echoedit_update(term->ldisc); break; case 20: /* LNM: Return sends ... */ term->cr_lf_return = state; @@ -3361,7 +3361,7 @@ static void term_out(Terminal *term) compatibility(VT100); power_on(term, TRUE); if (term->ldisc) /* cause ldisc to notice changes */ - ldisc_send(term->ldisc, NULL, 0, 0); + ldisc_echoedit_update(term->ldisc); if (term->reset_132) { if (!term->no_remote_resize) request_resize(term->frontend, 80, term->rows); diff --git a/unix/gtkwin.c b/unix/gtkwin.c index 4cce11e6..f3a17d58 100644 --- a/unix/gtkwin.c +++ b/unix/gtkwin.c @@ -194,7 +194,7 @@ int platform_default_i(const char *name, int def) } /* Dummy routine, only required in plink. */ -void ldisc_update(void *frontend, int echo, int edit) +void frontend_echoedit_update(void *frontend, int echo, int edit) { } @@ -3020,7 +3020,7 @@ void reset_terminal_menuitem(GtkMenuItem *item, gpointer data) struct gui_data *inst = (struct gui_data *)data; term_pwron(inst->term, TRUE); if (inst->ldisc) - ldisc_send(inst->ldisc, NULL, 0, 0); + ldisc_echoedit_update(inst->ldisc); } void copy_all_menuitem(GtkMenuItem *item, gpointer data) @@ -3088,7 +3088,7 @@ void change_settings_menuitem(GtkMenuItem *item, gpointer data) */ if (inst->ldisc) { ldisc_configure(inst->ldisc, inst->conf); - ldisc_send(inst->ldisc, NULL, 0, 0); + ldisc_echoedit_update(inst->ldisc); } /* Pass new config data to the terminal */ term_reconfig(inst->term, inst->conf); @@ -3924,7 +3924,7 @@ int pt_main(int argc, char **argv) start_backend(inst); - ldisc_send(inst->ldisc, NULL, 0, 0);/* cause ldisc to notice changes */ + ldisc_echoedit_update(inst->ldisc); /* cause ldisc to notice changes */ /* now we're reday to deal with the child exit handler being * called */ diff --git a/unix/uxplink.c b/unix/uxplink.c index 3901ba97..08780899 100644 --- a/unix/uxplink.c +++ b/unix/uxplink.c @@ -152,7 +152,7 @@ int term_ldisc(Terminal *term, int mode) { return FALSE; } -void ldisc_update(void *frontend, int echo, int edit) +void frontend_echoedit_update(void *frontend, int echo, int edit) { /* Update stdin read mode to reflect changes in line discipline. */ struct termios mode; @@ -178,8 +178,9 @@ void ldisc_update(void *frontend, int echo, int edit) mode.c_cc[VMIN] = 1; mode.c_cc[VTIME] = 0; /* FIXME: perhaps what we do with IXON/IXOFF should be an - * argument to ldisc_update(), to allow implementation of SSH-2 - * "xon-xoff" and Rlogin's equivalent? */ + * argument to frontend_echoedit_update(), to allow + * implementation of SSH-2 "xon-xoff" and Rlogin's + * equivalent? */ mode.c_iflag &= ~IXON; mode.c_iflag &= ~IXOFF; } @@ -981,7 +982,7 @@ int main(int argc, char **argv) */ local_tty = (tcgetattr(STDIN_FILENO, &orig_termios) == 0); atexit(cleanup_termios); - ldisc_update(NULL, 1, 1); + frontend_echoedit_update(NULL, 1, 1); sending = FALSE; now = GETTICKCOUNT(); diff --git a/windows/window.c b/windows/window.c index 7c320a0a..04059f06 100644 --- a/windows/window.c +++ b/windows/window.c @@ -219,7 +219,7 @@ const int share_can_be_downstream = TRUE; const int share_can_be_upstream = TRUE; /* Dummy routine, only required in plink. */ -void ldisc_update(void *frontend, int echo, int edit) +void frontend_echoedit_update(void *frontend, int echo, int edit) { } @@ -2236,7 +2236,7 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, */ if (ldisc) { ldisc_configure(ldisc, conf); - ldisc_send(ldisc, NULL, 0, 0); + ldisc_echoedit_update(ldisc); } if (pal) DeleteObject(pal); @@ -2379,7 +2379,7 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, case IDM_RESET: term_pwron(term, TRUE); if (ldisc) - ldisc_send(ldisc, NULL, 0, 0); + ldisc_echoedit_update(ldisc); break; case IDM_ABOUT: showabout(hwnd); diff --git a/windows/winplink.c b/windows/winplink.c index 43826622..3ede6b07 100644 --- a/windows/winplink.c +++ b/windows/winplink.c @@ -98,7 +98,7 @@ int term_ldisc(Terminal *term, int mode) { return FALSE; } -void ldisc_update(void *frontend, int echo, int edit) +void frontend_echoedit_update(void *frontend, int echo, int edit) { /* Update stdin read mode to reflect changes in line discipline. */ DWORD mode; -- 2.45.2