From 6cf6682c549a381e38e1895cb1c2d039cec6334f Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Tue, 13 Sep 2022 15:00:26 +0100 Subject: [PATCH] Rewrite some manual char-buffer-handling code. In the course of recent refactorings I noticed a couple of cases where we were doing old-fashioned preallocation of a char array with some conservative maximum size, then writing into it via *p++ or similar and hoping we got the calculation right. Now we have strbuf and dupcat, so we shouldn't ever have to do that. Fixed as many cases as I could find by searching for allocations of the form 'snewn(foo, char)'. Particularly worth a mention was the Windows GSSAPI setup code, which was directly using the Win32 Registry API, and looks much more legible using the windows/utils/registry.c wrappers. (But that was why I had to enhance them in the previous commit so as to be able to open registry keys read-only: without that, the open operation would actually fail on this key, which is not user-writable.) Also unix/askpass.c, which was doing a careful reallocation of its buffer to avoid secrets being left behind in the vacated memory - which is now just a matter of ensuring we called strbuf_new_nm(). --- otherbackends/telnet.c | 117 ++++++++++++++++++----------------------- pscp.c | 25 ++++----- settings.c | 60 ++++++--------------- ssh/scpserver.c | 17 +++--- terminal/terminal.c | 13 +++-- terminal/terminal.h | 3 +- unix/askpass.c | 63 +++++++--------------- unix/dialog.c | 4 +- unix/pty.c | 3 +- unix/storage.c | 12 ++--- windows/controls.c | 31 ++++------- windows/dialog.c | 30 +++-------- windows/gss.c | 87 ++++++++++++++---------------- 13 files changed, 174 insertions(+), 291 deletions(-) diff --git a/otherbackends/telnet.c b/otherbackends/telnet.c index 23b7fc9e..f43520a3 100644 --- a/otherbackends/telnet.c +++ b/otherbackends/telnet.c @@ -358,28 +358,26 @@ static void proc_rec_opt(Telnet *telnet, int cmd, int option) static void process_subneg(Telnet *telnet) { - unsigned char *b, *p, *q; - int var, value, n, bsize; - char *e, *eval, *ekey, *user; + unsigned char *p, *q; + int var, value; switch (telnet->sb_opt) { case TELOPT_TSPEED: if (telnet->sb_buf->len == 1 && telnet->sb_buf->u[0] == TELQUAL_SEND) { char *termspeed = conf_get_str(telnet->conf, CONF_termspeed); - b = snewn(20 + strlen(termspeed), unsigned char); - b[0] = IAC; - b[1] = SB; - b[2] = TELOPT_TSPEED; - b[3] = TELQUAL_IS; - strcpy((char *)(b + 4), termspeed); - n = 4 + strlen(termspeed); - b[n] = IAC; - b[n + 1] = SE; - telnet->bufsize = sk_write(telnet->s, b, n + 2); + strbuf *sb = strbuf_new(); + put_byte(sb, IAC); + put_byte(sb, SB); + put_byte(sb, TELOPT_TSPEED); + put_byte(sb, TELQUAL_IS); + put_datapl(sb, ptrlen_from_asciz(termspeed)); + put_byte(sb, IAC); + put_byte(sb, SE); + telnet->bufsize = sk_write(telnet->s, sb->s, sb->len); logevent(telnet->logctx, "server subnegotiation: SB TSPEED SEND"); logeventf(telnet->logctx, "client subnegotiation: SB TSPEED IS %s", termspeed); - sfree(b); + strbuf_free(sb); } else logevent(telnet->logctx, "server subnegotiation: SB TSPEED "); @@ -387,24 +385,24 @@ static void process_subneg(Telnet *telnet) case TELOPT_TTYPE: if (telnet->sb_buf->len == 1 && telnet->sb_buf->u[0] == TELQUAL_SEND) { char *termtype = conf_get_str(telnet->conf, CONF_termtype); - b = snewn(20 + strlen(termtype), unsigned char); - b[0] = IAC; - b[1] = SB; - b[2] = TELOPT_TTYPE; - b[3] = TELQUAL_IS; - for (n = 0; termtype[n]; n++) - b[n + 4] = (termtype[n] >= 'a' && termtype[n] <= 'z' ? - termtype[n] + 'A' - 'a' : - termtype[n]); - b[n + 4] = IAC; - b[n + 5] = SE; - telnet->bufsize = sk_write(telnet->s, b, n + 6); - b[n + 4] = 0; - logevent(telnet->logctx, - "server subnegotiation: SB TTYPE SEND"); - logeventf(telnet->logctx, - "client subnegotiation: SB TTYPE IS %s", b + 4); - sfree(b); + strbuf *sb = strbuf_new(); + put_byte(sb, IAC); + put_byte(sb, SB); + put_byte(sb, TELOPT_TTYPE); + put_byte(sb, TELQUAL_IS); + size_t tt_start = sb->len; + for (size_t n = 0; termtype[n]; n++) + put_byte(sb, (termtype[n] >= 'a' && termtype[n] <= 'z' ? + termtype[n] + 'A' - 'a' : termtype[n])); + size_t tt_end = sb->len; + put_byte(sb, IAC); + put_byte(sb, SE); + telnet->bufsize = sk_write(telnet->s, sb->s, sb->len); + strbuf_shrink_to(sb, tt_end); + logevent(telnet->logctx, "server subnegotiation: SB TTYPE SEND"); + logeventf(telnet->logctx, "client subnegotiation: SB TTYPE IS %s", + sb->s + tt_start); + strbuf_free(sb); } else logevent(telnet->logctx, "server subnegotiation: SB TTYPE \r\n"); @@ -446,49 +444,34 @@ static void process_subneg(Telnet *telnet) value = RFC_VALUE; var = RFC_VAR; } - bsize = 20; - for (eval = conf_get_str_strs(telnet->conf, CONF_environmt, - NULL, &ekey); - eval != NULL; - eval = conf_get_str_strs(telnet->conf, CONF_environmt, - ekey, &ekey)) - bsize += strlen(ekey) + strlen(eval) + 2; - user = get_remote_username(telnet->conf); - if (user) - bsize += 6 + strlen(user); - b = snewn(bsize, unsigned char); - b[0] = IAC; - b[1] = SB; - b[2] = telnet->sb_opt; - b[3] = TELQUAL_IS; - n = 4; + strbuf *sb = strbuf_new(); + put_byte(sb, IAC); + put_byte(sb, SB); + put_byte(sb, telnet->sb_opt); + put_byte(sb, TELQUAL_IS); + char *ekey, *eval; for (eval = conf_get_str_strs(telnet->conf, CONF_environmt, NULL, &ekey); eval != NULL; eval = conf_get_str_strs(telnet->conf, CONF_environmt, ekey, &ekey)) { - b[n++] = var; - for (e = ekey; *e; e++) - b[n++] = *e; - b[n++] = value; - for (e = eval; *e; e++) - b[n++] = *e; + put_byte(sb, var); + put_datapl(sb, ptrlen_from_asciz(ekey)); + put_byte(sb, value); + put_datapl(sb, ptrlen_from_asciz(eval)); } + char *user = get_remote_username(telnet->conf); if (user) { - b[n++] = var; - b[n++] = 'U'; - b[n++] = 'S'; - b[n++] = 'E'; - b[n++] = 'R'; - b[n++] = value; - for (e = user; *e; e++) - b[n++] = *e; + put_byte(sb, var); + put_datalit(sb, "USER"); + put_byte(sb, value); + put_datapl(sb, ptrlen_from_asciz(user)); } - b[n++] = IAC; - b[n++] = SE; - telnet->bufsize = sk_write(telnet->s, b, n); - if (n == 6) { + put_byte(sb, IAC); + put_byte(sb, SE); + telnet->bufsize = sk_write(telnet->s, sb->s, sb->len); + if (sb->len == 6) { logeventf(telnet->logctx, "client subnegotiation: SB %s IS ", telopt(telnet->sb_opt)); @@ -505,7 +488,7 @@ static void process_subneg(Telnet *telnet) if (user) logeventf(telnet->logctx, " USER=%s", user); } - sfree(b); + strbuf_free(sb); sfree(user); } break; diff --git a/pscp.c b/pscp.c index 87fb1722..d5ac8845 100644 --- a/pscp.c +++ b/pscp.c @@ -2120,7 +2120,6 @@ static void get_dir_list(int argc, char *argv[]) { char *wsrc, *host, *user; const char *src; - char *cmd, *p; const char *q; char c; @@ -2150,24 +2149,18 @@ static void get_dir_list(int argc, char *argv[]) user = NULL; } - cmd = snewn(4 * strlen(src) + 100, char); - strcpy(cmd, "ls -la '"); - p = cmd + strlen(cmd); + strbuf *cmd = strbuf_new(); + put_datalit(cmd, "ls -la '"); for (q = src; *q; q++) { - if (*q == '\'') { - *p++ = '\''; - *p++ = '\\'; - *p++ = '\''; - *p++ = '\''; - } else { - *p++ = *q; - } + if (*q == '\'') + put_datalit(cmd, "'\\''"); + else + put_byte(cmd, *q); } - *p++ = '\''; - *p = '\0'; + put_datalit(cmd, "'"); - do_cmd(host, user, cmd); - sfree(cmd); + do_cmd(host, user, cmd->s); + strbuf_free(cmd); if (using_sftp) { scp_sftp_listdir(src); diff --git a/settings.c b/settings.c index cd286eb4..046f176e 100644 --- a/settings.c +++ b/settings.c @@ -267,19 +267,10 @@ static bool gppmap(settings_r *sesskey, const char *name, static void wmap(settings_w *sesskey, char const *outkey, Conf *conf, int primary, bool include_values) { - char *buf, *p, *key, *realkey; + char *key, *realkey; const char *val, *q; - int len; - len = 1; /* allow for NUL */ - - for (val = conf_get_str_strs(conf, primary, NULL, &key); - val != NULL; - val = conf_get_str_strs(conf, primary, key, &key)) - len += 2 + 2 * (strlen(key) + strlen(val)); /* allow for escaping */ - - buf = snewn(len, char); - p = buf; + strbuf *sb = strbuf_new(); for (val = conf_get_str_strs(conf, primary, NULL, &key); val != NULL; @@ -304,19 +295,19 @@ static void wmap(settings_w *sesskey, char const *outkey, Conf *conf, realkey = NULL; } - if (p != buf) - *p++ = ','; + if (sb->len) + put_byte(sb, ','); for (q = key; *q; q++) { if (*q == '=' || *q == ',' || *q == '\\') - *p++ = '\\'; - *p++ = *q; + put_byte(sb, '\\'); + put_byte(sb, *q); } if (include_values) { - *p++ = '='; + put_byte(sb, '='); for (q = val; *q; q++) { if (*q == '=' || *q == ',' || *q == '\\') - *p++ = '\\'; - *p++ = *q; + put_byte(sb, '\\'); + put_byte(sb, *q); } } @@ -325,9 +316,8 @@ static void wmap(settings_w *sesskey, char const *outkey, Conf *conf, key = realkey; } } - *p = '\0'; - write_setting_s(sesskey, outkey, buf); - sfree(buf); + write_setting_s(sesskey, outkey, sb->s); + strbuf_free(sb); } static int key2val(const struct keyvalwhere *mapping, @@ -456,34 +446,18 @@ static void wprefs(settings_w *sesskey, const char *name, const struct keyvalwhere *mapping, int nvals, Conf *conf, int primary) { - char *buf, *p; - int i, maxlen; + strbuf *sb = strbuf_new(); - for (maxlen = i = 0; i < nvals; i++) { + for (int i = 0; i < nvals; i++) { const char *s = val2key(mapping, nvals, conf_get_int_int(conf, primary, i)); - if (s) { - maxlen += (maxlen > 0 ? 1 : 0) + strlen(s); - } + if (s) + put_fmt(sb, "%s%s", (sb->len ? "," : ""), s); } - buf = snewn(maxlen + 1, char); - p = buf; + write_setting_s(sesskey, name, sb->s); - for (i = 0; i < nvals; i++) { - const char *s = val2key(mapping, nvals, - conf_get_int_int(conf, primary, i)); - if (s) { - p += sprintf(p, "%s%s", (p > buf ? "," : ""), s); - } - } - - assert(p - buf == maxlen); - *p = '\0'; - - write_setting_s(sesskey, name, buf); - - sfree(buf); + strbuf_free(sb); } static void write_setting_b(settings_w *handle, const char *key, bool value) diff --git a/ssh/scpserver.c b/ssh/scpserver.c index 15633ecb..0995c998 100644 --- a/ssh/scpserver.c +++ b/ssh/scpserver.c @@ -799,20 +799,17 @@ static void scp_source_process_stack(ScpSource *scp) scp->head = node; /* put back the unfinished READDIR */ node = NULL; /* and prevent it being freed */ } else { - ptrlen subpath; - subpath.len = node->pathname.len + 1 + scp->reply.name.len; - char *subpath_space = snewn(subpath.len, char); - subpath.ptr = subpath_space; - memcpy(subpath_space, node->pathname.ptr, node->pathname.len); - subpath_space[node->pathname.len] = '/'; - memcpy(subpath_space + node->pathname.len + 1, - scp->reply.name.ptr, scp->reply.name.len); + strbuf *subpath = strbuf_new(); + put_datapl(subpath, node->pathname); + put_byte(subpath, '/'); + put_datapl(subpath, scp->reply.name); scp->head = node; /* put back the unfinished READDIR */ node = NULL; /* and prevent it being freed */ - scp_source_push_name(scp, subpath, scp->reply.attrs, NULL); + scp_source_push_name(scp, ptrlen_from_strbuf(subpath), + scp->reply.attrs, NULL); - sfree(subpath_space); + strbuf_free(subpath); } } else if (node->attrs.permissions & PERMS_DIRECTORY) { assert(scp->recursive || node->wildcard); diff --git a/terminal/terminal.c b/terminal/terminal.c index ee227190..01feecb8 100644 --- a/terminal/terminal.c +++ b/terminal/terminal.c @@ -1588,19 +1588,17 @@ static void term_copy_stuff_from_conf(Terminal *term) */ { char *answerback = conf_get_str(term->conf, CONF_answerback); - int maxlen = strlen(answerback); - term->answerback = snewn(maxlen, char); - term->answerbacklen = 0; + strbuf_clear(term->answerback); while (*answerback) { char *n; char c = ctrlparse(answerback, &n); if (n) { - term->answerback[term->answerbacklen++] = c; + put_byte(term->answerback, c); answerback = n; } else { - term->answerback[term->answerbacklen++] = *answerback++; + put_byte(term->answerback, *answerback++); } } } @@ -1980,6 +1978,7 @@ Terminal *term_init(Conf *myconf, struct unicode_data *ucsdata, TermWin *win) term->termstate = TOPLEVEL; term->selstate = NO_SELECTION; term->curstype = 0; + term->answerback = strbuf_new(); term_copy_stuff_from_conf(term); @@ -2095,7 +2094,7 @@ void term_free(Terminal *term) sfree(term->ltemp); sfree(term->wcFrom); sfree(term->wcTo); - sfree(term->answerback); + strbuf_free(term->answerback); for (i = 0; i < term->bidi_cache_size; i++) { sfree(term->pre_bidi_cache[i].chars); @@ -3807,7 +3806,7 @@ static void term_out(Terminal *term, bool called_from_term_data) if (term->ldisc) { strbuf *buf = term_input_data_from_charset( term, DEFAULT_CODEPAGE, - term->answerback, term->answerbacklen); + term->answerback->s, term->answerback->len); ldisc_send(term->ldisc, buf->s, buf->len, false); strbuf_free(buf); } diff --git a/terminal/terminal.h b/terminal/terminal.h index 3f918b22..c9d93342 100644 --- a/terminal/terminal.h +++ b/terminal/terminal.h @@ -300,8 +300,7 @@ struct terminal_tag { * the former every time. */ bool ansi_colour; - char *answerback; - int answerbacklen; + strbuf *answerback; bool no_arabicshaping; int beep; bool bellovl; diff --git a/unix/askpass.c b/unix/askpass.c index b45a1c23..a1143a8f 100644 --- a/unix/askpass.c +++ b/unix/askpass.c @@ -46,8 +46,7 @@ struct askpass_ctx { GdkColor cols[3]; #endif char *error_message; /* if we finish without a passphrase */ - char *passphrase; /* if we finish with one */ - int passlen, passsize; + strbuf *passphrase; /* if we finish with one */ #if GTK_CHECK_VERSION(3,20,0) GdkSeat *seat; /* for gdk_seat_grab */ #elif GTK_CHECK_VERSION(3,0,0) @@ -107,48 +106,30 @@ static void visually_acknowledge_keypress(struct askpass_ctx *ctx) ctx->active_area = new_active; } -static int last_char_len(struct askpass_ctx *ctx) +static size_t last_char_start(struct askpass_ctx *ctx) { /* * GTK always encodes in UTF-8, so we can do this in a fixed way. */ - int i; - assert(ctx->passlen > 0); - i = ctx->passlen - 1; - while ((unsigned)((unsigned char)ctx->passphrase[i] - 0x80) < 0x40) { + assert(ctx->passphrase->len > 0); + size_t i = ctx->passphrase->len - 1; + while ((unsigned)(ctx->passphrase->u[i] - 0x80) < 0x40) { if (i == 0) break; i--; } - return ctx->passlen - i; + return i; } static void add_text_to_passphrase(struct askpass_ctx *ctx, gchar *str) { - int len = strlen(str); - if (ctx->passlen + len >= ctx->passsize) { - /* Take some care with buffer expansion, because there are - * pieces of passphrase in the old buffer so we should ensure - * realloc doesn't leave a copy lying around in the address - * space. */ - int oldsize = ctx->passsize; - char *newbuf; - - ctx->passsize = (ctx->passlen + len) * 5 / 4 + 1024; - newbuf = snewn(ctx->passsize, char); - memcpy(newbuf, ctx->passphrase, oldsize); - smemclr(ctx->passphrase, oldsize); - sfree(ctx->passphrase); - ctx->passphrase = newbuf; - } - strcpy(ctx->passphrase + ctx->passlen, str); - ctx->passlen += len; + put_datapl(ctx->passphrase, ptrlen_from_asciz(str)); visually_acknowledge_keypress(ctx); } static void cancel_askpass(struct askpass_ctx *ctx, const char *msg) { - smemclr(ctx->passphrase, ctx->passsize); + strbuf_free(ctx->passphrase); ctx->passphrase = NULL; ctx->error_message = dupstr(msg); gtk_main_quit(); @@ -182,7 +163,7 @@ static gint key_event(GtkWidget *widget, GdkEventKey *event, gpointer data) if (event->type == GDK_KEY_PRESS) { if (!strcmp(event->string, "\x15")) { /* Ctrl-U. Wipe out the whole line */ - ctx->passlen = 0; + strbuf_clear(ctx->passphrase); visually_acknowledge_keypress(ctx); } else if (!strcmp(event->string, "\x17")) { /* Ctrl-W. Delete back to the last space->nonspace @@ -190,20 +171,21 @@ static gint key_event(GtkWidget *widget, GdkEventKey *event, gpointer data) * way (mimicking terminal drivers), and don't attempt * to second-guess exciting Unicode space * characters. */ - while (ctx->passlen > 0) { + while (ctx->passphrase->len > 0) { char deleted, prior; - ctx->passlen -= last_char_len(ctx); - deleted = ctx->passphrase[ctx->passlen]; - prior = (ctx->passlen == 0 ? ' ' : - ctx->passphrase[ctx->passlen-1]); + size_t newlen = last_char_start(ctx); + deleted = ctx->passphrase->s[newlen]; + strbuf_shrink_to(ctx->passphrase, newlen); + prior = (ctx->passphrase->len == 0 ? ' ' : + ctx->passphrase->s[ctx->passphrase->len-1]); if (!g_ascii_isspace(deleted) && g_ascii_isspace(prior)) break; } visually_acknowledge_keypress(ctx); } else if (event->keyval == GDK_KEY_BackSpace) { /* Backspace. Delete one character. */ - if (ctx->passlen > 0) - ctx->passlen -= last_char_len(ctx); + if (ctx->passphrase->len > 0) + strbuf_shrink_to(ctx->passphrase, last_char_start(ctx)); visually_acknowledge_keypress(ctx); #if !GTK_CHECK_VERSION(2,0,0) } else if (event->string[0]) { @@ -427,9 +409,7 @@ static const char *gtk_askpass_setup(struct askpass_ctx *ctx, int i; GtkBox *action_area; - ctx->passlen = 0; - ctx->passsize = 2048; - ctx->passphrase = snewn(ctx->passsize, char); + ctx->passphrase = strbuf_new_nm(); /* * Create widgets. @@ -553,11 +533,6 @@ static void gtk_askpass_cleanup(struct askpass_ctx *ctx) #endif gtk_grab_remove(ctx->promptlabel); - if (ctx->passphrase) { - assert(ctx->passlen < ctx->passsize); - ctx->passphrase[ctx->passlen] = '\0'; - } - gtk_widget_destroy(ctx->dialog); } @@ -612,7 +587,7 @@ char *gtk_askpass_main(const char *display, const char *wintitle, if (ctx->passphrase) { *success = true; - return ctx->passphrase; + return strbuf_to_str(ctx->passphrase); } else { *success = false; return ctx->error_message; diff --git a/unix/dialog.c b/unix/dialog.c index 2ff51ede..c73cdaa3 100644 --- a/unix/dialog.c +++ b/unix/dialog.c @@ -594,9 +594,7 @@ void dlg_listbox_addwithid(dlgcontrol *ctrl, dlgparam *dp, cols = cols ? cols : 1; for (i = 0; i < cols; i++) { int collen = strcspn(text, "\t"); - char *tmpstr = snewn(collen+1, char); - memcpy(tmpstr, text, collen); - tmpstr[collen] = '\0'; + char *tmpstr = mkstr(make_ptrlen(text, collen)); gtk_list_store_set(uc->listmodel, &iter, i+1, tmpstr, -1); sfree(tmpstr); text += collen; diff --git a/unix/pty.c b/unix/pty.c index 05c58929..c03328f7 100644 --- a/unix/pty.c +++ b/unix/pty.c @@ -1228,9 +1228,8 @@ Backend *pty_backend_create( char *shellname; if (conf_get_bool(conf, CONF_login_shell)) { const char *p = strrchr(shell, '/'); - shellname = snewn(2+strlen(shell), char); p = p ? p+1 : shell; - sprintf(shellname, "-%s", p); + shellname = dupprintf("-%s", p); } else shellname = (char *)shell; execl(shell, shellname, (void *)NULL); diff --git a/unix/storage.c b/unix/storage.c index 35a276c2..042887f2 100644 --- a/unix/storage.c +++ b/unix/storage.c @@ -321,7 +321,6 @@ static int keycmp(void *av, void *bv) void provide_xrm_string(const char *string, const char *progname) { const char *p, *q; - char *key; struct skeyval *xrms, *ret; p = q = strchr(string, ':'); @@ -330,14 +329,13 @@ void provide_xrm_string(const char *string, const char *progname) " \"%s\"\n", progname, string); return; } - q++; + xrms = snew(struct skeyval); + while (p > string && p[-1] != '.' && p[-1] != '*') p--; - xrms = snew(struct skeyval); - key = snewn(q-p, char); - memcpy(key, p, q-p); - key[q-p-1] = '\0'; - xrms->key = key; + xrms->key = mkstr(make_ptrlen(p, q-p)); + + q++; while (*q && isspace((unsigned char)*q)) q++; xrms->value = dupstr(q); diff --git a/windows/controls.c b/windows/controls.c index ce3638e4..8f14796e 100644 --- a/windows/controls.c +++ b/windows/controls.c @@ -393,13 +393,11 @@ char *staticwrap(struct ctlpos *cp, HWND hwnd, const char *text, int *lines) INT *pwidths, nfit; SIZE size; const char *p; - char *ret, *q; RECT r; HFONT oldfont, newfont; - ret = snewn(1+strlen(text), char); + strbuf *sb = strbuf_new(); p = text; - q = ret; pwidths = snewn(1+strlen(text), INT); /* @@ -432,7 +430,7 @@ char *staticwrap(struct ctlpos *cp, HWND hwnd, const char *text, int *lines) * Either way, we stop wrapping, copy the remainder of * the input string unchanged to the output, and leave. */ - strcpy(q, p); + put_datapl(sb, ptrlen_from_asciz(p)); break; } @@ -449,9 +447,8 @@ char *staticwrap(struct ctlpos *cp, HWND hwnd, const char *text, int *lines) } } - strncpy(q, p, nfit); - q[nfit] = '\n'; - q += nfit+1; + put_data(sb, p, nfit); + put_byte(sb, '\n'); p += nfit; while (*p && isspace((unsigned char)*p)) @@ -467,7 +464,7 @@ char *staticwrap(struct ctlpos *cp, HWND hwnd, const char *text, int *lines) sfree(pwidths); - return ret; + return strbuf_to_str(sb); } /* @@ -1181,30 +1178,24 @@ void progressbar(struct ctlpos *cp, int id) */ static char *shortcut_escape(const char *text, char shortcut) { - char *ret; - char const *p; - char *q; - if (!text) return NULL; /* sfree won't choke on this */ - ret = snewn(2*strlen(text)+1, char); /* size potentially doubles! */ + strbuf *sb = strbuf_new(); shortcut = tolower((unsigned char)shortcut); - p = text; - q = ret; + const char *p = text; while (*p) { if (shortcut != NO_SHORTCUT && tolower((unsigned char)*p) == shortcut) { - *q++ = '&'; + put_byte(sb, '&'); shortcut = NO_SHORTCUT; /* stop it happening twice */ } else if (*p == '&') { - *q++ = '&'; + put_byte(sb, '&'); } - *q++ = *p++; + put_byte(sb, *p++); } - *q = '\0'; - return ret; + return strbuf_to_str(sb); } void winctrl_add_shortcuts(struct dlgparam *dp, struct winctrl *c) diff --git a/windows/dialog.c b/windows/dialog.c index 68d88df8..b97c4031 100644 --- a/windows/dialog.c +++ b/windows/dialog.c @@ -300,35 +300,21 @@ static INT_PTR CALLBACK LogProc(HWND hwnd, UINT msg, LB_GETSELITEMS, selcount, (LPARAM) selitems); - int i; - int size; - char *clipdata; - static unsigned char sel_nl[] = SEL_NL; + static const unsigned char sel_nl[] = SEL_NL; if (count == 0) { /* can't copy zero stuff */ MessageBeep(0); break; } - size = 0; - for (i = 0; i < count; i++) - size += - strlen(getevent(selitems[i])) + sizeof(sel_nl); - - clipdata = snewn(size, char); - if (clipdata) { - char *p = clipdata; - for (i = 0; i < count; i++) { - char *q = getevent(selitems[i]); - int qlen = strlen(q); - memcpy(p, q, qlen); - p += qlen; - memcpy(p, sel_nl, sizeof(sel_nl)); - p += sizeof(sel_nl); - } - write_aclip(hwnd, CLIP_SYSTEM, clipdata, size); - sfree(clipdata); + strbuf *sb = strbuf_new(); + for (int i = 0; i < count; i++) { + char *q = getevent(selitems[i]); + put_datapl(sb, ptrlen_from_asciz(q)); + put_data(sb, sel_nl, sizeof(sel_nl)); } + write_aclip(hwnd, CLIP_SYSTEM, sb->s, sb->len); + strbuf_free(sb); sfree(selitems); for (i = 0; i < (ninitial + ncircular); i++) diff --git a/windows/gss.c b/windows/gss.c index 724eeec1..60af74c0 100644 --- a/windows/gss.c +++ b/windows/gss.c @@ -118,7 +118,6 @@ static void add_library_to_never_unload_tree(HMODULE module) struct ssh_gss_liblist *ssh_gss_setup(Conf *conf) { HMODULE module; - HKEY regkey; struct ssh_gss_liblist *list = snew(struct ssh_gss_liblist); char *path; static HMODULE kernel32_module; @@ -137,55 +136,47 @@ struct ssh_gss_liblist *ssh_gss_setup(Conf *conf) /* MIT Kerberos GSSAPI implementation */ module = NULL; - if (RegOpenKey(HKEY_LOCAL_MACHINE, "SOFTWARE\\MIT\\Kerberos", ®key) - == ERROR_SUCCESS) { - DWORD type, size; - LONG ret; - char *buffer; - - /* Find out the string length */ - ret = RegQueryValueEx(regkey, "InstallDir", NULL, &type, NULL, &size); - - if (ret == ERROR_SUCCESS && type == REG_SZ) { - buffer = snewn(size + 20, char); - ret = RegQueryValueEx(regkey, "InstallDir", NULL, - &type, (LPBYTE)buffer, &size); - if (ret == ERROR_SUCCESS && type == REG_SZ) { - strcat (buffer, "\\bin"); - if(p_AddDllDirectory) { - /* Add MIT Kerberos' path to the DLL search path, - * it loads its own DLLs further down the road */ - wchar_t *dllPath = - dup_mb_to_wc(DEFAULT_CODEPAGE, 0, buffer); - p_AddDllDirectory(dllPath); - sfree(dllPath); - } - strcat (buffer, "\\gssapi"MIT_KERB_SUFFIX".dll"); - module = LoadLibraryEx (buffer, NULL, - LOAD_LIBRARY_SEARCH_SYSTEM32 | - LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | - LOAD_LIBRARY_SEARCH_USER_DIRS); - - /* - * The MIT Kerberos DLL suffers an internal segfault - * for some reason if you unload and reload one within - * the same process. So, make sure that after we load - * this library, we never free it. - * - * Or rather: after we've loaded it once, if any - * _further_ load returns the same module handle, we - * immediately free it again (to prevent the Windows - * API's internal reference count growing without - * bound). But on the other hand we never free it in - * ssh_gss_cleanup. - */ - if (library_is_in_never_unload_tree(module)) - FreeLibrary(module); - add_library_to_never_unload_tree(module); + HKEY regkey = open_regkey_ro(HKEY_LOCAL_MACHINE, + "SOFTWARE\\MIT\\Kerberos"); + if (regkey) { + char *installdir = get_reg_sz(regkey, "InstallDir"); + if (installdir) { + char *bindir = dupcat(installdir, "\\bin"); + if(p_AddDllDirectory) { + /* Add MIT Kerberos' path to the DLL search path, + * it loads its own DLLs further down the road */ + wchar_t *dllPath = dup_mb_to_wc(DEFAULT_CODEPAGE, 0, bindir); + p_AddDllDirectory(dllPath); + sfree(dllPath); } - sfree(buffer); + + char *dllfile = dupcat(bindir, "\\gssapi"MIT_KERB_SUFFIX".dll"); + module = LoadLibraryEx(dllfile, NULL, + LOAD_LIBRARY_SEARCH_SYSTEM32 | + LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | + LOAD_LIBRARY_SEARCH_USER_DIRS); + + /* + * The MIT Kerberos DLL suffers an internal segfault for + * some reason if you unload and reload one within the + * same process. So, make sure that after we load this + * library, we never free it. + * + * Or rather: after we've loaded it once, if any _further_ + * load returns the same module handle, we immediately + * free it again (to prevent the Windows API's internal + * reference count growing without bound). But on the + * other hand we never free it in ssh_gss_cleanup. + */ + if (library_is_in_never_unload_tree(module)) + FreeLibrary(module); + add_library_to_never_unload_tree(module); + + sfree(dllfile); + sfree(bindir); + sfree(installdir); } - RegCloseKey(regkey); + close_regkey(regkey); } if (module) { struct ssh_gss_library *lib =