From a990738aca6eafb5b3daba3658872950ab81ddbe Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Thu, 24 May 2018 10:48:20 +0100 Subject: [PATCH] Use the BinarySink system for conf serialisation. Now instead of iterating through conf twice in separate functions, once to count up the size of the serialised data and once to write it out, I just go through once and dump it all in a strbuf. (Of course, I could still do a two-pass count-then-allocate approach easily enough in this system; nothing would stop me writing a BinarySink implementation that didn't actually store any data and just counted its size, and then I could choose at each call site whether I preferred to do it that way.) --- Recipe | 17 +++++++----- conf.c | 67 +++++++---------------------------------------- putty.h | 8 +++--- unix/gtkmain.c | 32 +++++++++------------- unix/uxmisc.c | 16 +++-------- windows/window.c | 9 +++++-- windows/winmisc.c | 24 +++++------------ 7 files changed, 54 insertions(+), 119 deletions(-) diff --git a/Recipe b/Recipe index 83c986fc..77dd2bc5 100644 --- a/Recipe +++ b/Recipe @@ -229,9 +229,12 @@ CFLAGS += -DWINVER=0x0500 -D_WIN32_WINDOWS=0x0410 -D_WIN32_WINNT=0x0500 # names. A line beginning `+' is assumed to continue the previous # line. +# conf.c and its dependencies. +CONF = conf marshal + # Terminal emulator and its (platform-independent) dependencies. TERMINAL = terminal wcwidth ldiscucs logging tree234 minibidi - + config dialog conf + + config dialog CONF # GUI front end and terminal emulator (putty, puttytel). GUITERM = TERMINAL window windlg winctrls sizetip winprint winutils @@ -261,7 +264,7 @@ SFTP = sftp int64 logging cmdline # Miscellaneous objects appearing in all the utilities, or all the # network ones, or the Unix or Windows subsets of those in turn. MISC = misc marshal -MISCNET = timing callback MISC version settings tree234 proxy conf be_misc +MISCNET = timing callback MISC version settings tree234 proxy CONF be_misc WINMISC = MISCNET winstore winnet winhandl cmdline windefs winmisc winproxy + wintime winhsock errsock winsecur winucs miscucs UXMISC = MISCNET uxstore uxsel uxnet uxpeer uxmisc uxproxy time @@ -312,7 +315,7 @@ pageant : [G] winpgnt pageant sshrsa sshpubk sshdes sshbn sshmd5 version puttygen : [G] winpgen sshrsag sshdssg sshprime sshdes sshbn sshmd5 version + sshrand winnoise sshsha winstore MISC winctrls sshrsa sshdss winmisc + sshpubk sshaes sshsh256 sshsh512 IMPORT winutils puttygen.res - + tree234 notiming winhelp winnojmp conf LIBS wintime sshecc + + tree234 notiming winhelp winnojmp CONF LIBS wintime sshecc + sshecdsag winsecur pterm : [X] GTKTERM uxmisc misc ldisc settings uxpty uxsel BE_NONE uxstore @@ -331,7 +334,7 @@ plink : [U] uxplink uxcons NONSSH UXSSH U_BE_ALL logging UXMISC uxsignal PUTTYGEN_UNIX = sshrsag sshdssg sshprime sshdes sshbn sshmd5 version + sshrand uxnoise sshsha MISC sshrsa sshdss uxcons uxstore uxmisc + sshpubk sshaes sshsh256 sshsh512 IMPORT puttygen.res time tree234 - + uxgen notiming conf sshecc sshecdsag uxnogtk + + uxgen notiming CONF sshecc sshecdsag uxnogtk puttygen : [U] cmdgen PUTTYGEN_UNIX cgtest : [UT] cgtest PUTTYGEN_UNIX @@ -340,7 +343,7 @@ psftp : [U] psftp uxsftp uxcons UXSSH BE_SSH SFTP wildcard UXMISC uxnogtk pageant : [X] uxpgnt uxagentc aqsync pageant sshrsa sshpubk sshdes sshbn + sshmd5 version tree234 misc sshaes sshsha sshdss sshsh256 sshsh512 - + sshecc conf uxsignal nocproxy nogss be_none x11fwd ux_x11 uxcons + + sshecc CONF uxsignal nocproxy nogss be_none x11fwd ux_x11 uxcons + gtkask gtkmisc UXMISC ptermapp : [XT] GTKTERM uxmisc misc ldisc settings uxpty uxsel BE_NONE uxstore @@ -353,8 +356,8 @@ osxlaunch : [UT] osxlaunch fuzzterm : [UT] UXTERM CHARSET misc version uxmisc uxucs fuzzterm time settings + uxstore be_none uxnogtk -testbn : [UT] testbn sshbn MISC version conf tree234 uxmisc uxnogtk -testbn : [C] testbn sshbn MISC version conf tree234 winmisc LIBS +testbn : [UT] testbn sshbn MISC version CONF tree234 uxmisc uxnogtk +testbn : [C] testbn sshbn MISC version CONF tree234 winmisc LIBS # ---------------------------------------------------------------------- # On Windows, provide a means of removing local test binaries that we diff --git a/conf.c b/conf.c index 92341758..89d6412b 100644 --- a/conf.c +++ b/conf.c @@ -469,86 +469,39 @@ void conf_set_fontspec(Conf *conf, int primary, const FontSpec *value) conf_insert(conf, entry); } -int conf_serialised_size(Conf *conf) +void conf_serialise(BinarySink *bs, Conf *conf) { int i; struct conf_entry *entry; - int size = 0; for (i = 0; (entry = index234(conf->tree, i)) != NULL; i++) { - size += 4; /* primary key */ + put_uint32(bs, entry->key.primary); + switch (subkeytypes[entry->key.primary]) { case TYPE_INT: - size += 4; + put_uint32(bs, entry->key.secondary.i); break; case TYPE_STR: - size += 1 + strlen(entry->key.secondary.s); + put_asciz(bs, entry->key.secondary.s); break; } switch (valuetypes[entry->key.primary]) { case TYPE_INT: - size += 4; + put_uint32(bs, entry->value.u.intval); break; case TYPE_STR: - size += 1 + strlen(entry->value.u.stringval); + put_asciz(bs, entry->value.u.stringval); break; case TYPE_FILENAME: - size += filename_serialise(entry->value.u.fileval, NULL); + filename_serialise(bs, entry->value.u.fileval); break; case TYPE_FONT: - size += fontspec_serialise(entry->value.u.fontval, NULL); + fontspec_serialise(bs, entry->value.u.fontval); break; } } - size += 4; /* terminator value */ - - return size; -} - -void conf_serialise(Conf *conf, void *vdata) -{ - unsigned char *data = (unsigned char *)vdata; - int i, len; - struct conf_entry *entry; - - for (i = 0; (entry = index234(conf->tree, i)) != NULL; i++) { - PUT_32BIT_MSB_FIRST(data, entry->key.primary); - data += 4; - - switch (subkeytypes[entry->key.primary]) { - case TYPE_INT: - PUT_32BIT_MSB_FIRST(data, entry->key.secondary.i); - data += 4; - break; - case TYPE_STR: - len = strlen(entry->key.secondary.s); - memcpy(data, entry->key.secondary.s, len); - data += len; - *data++ = 0; - break; - } - switch (valuetypes[entry->key.primary]) { - case TYPE_INT: - PUT_32BIT_MSB_FIRST(data, entry->value.u.intval); - data += 4; - break; - case TYPE_STR: - len = strlen(entry->value.u.stringval); - memcpy(data, entry->value.u.stringval, len); - data += len; - *data++ = 0; - break; - case TYPE_FILENAME: - data += filename_serialise(entry->value.u.fileval, data); - break; - case TYPE_FONT: - data += fontspec_serialise(entry->value.u.fontval, data); - break; - } - } - - PUT_32BIT_MSB_FIRST(data, 0xFFFFFFFFU); + put_uint32(bs, 0xFFFFFFFFU); } int conf_deserialise(Conf *conf, void *vdata, int maxsize) diff --git a/putty.h b/putty.h index 139ed5d6..72f0c164 100644 --- a/putty.h +++ b/putty.h @@ -20,6 +20,7 @@ #include "puttyps.h" #include "network.h" #include "misc.h" +#include "marshal.h" /* * We express various time intervals in unsigned long minutes, but may need to @@ -1013,8 +1014,7 @@ void conf_del_str_str(Conf *conf, int key, const char *subkey); void conf_set_filename(Conf *conf, int key, const Filename *val); void conf_set_fontspec(Conf *conf, int key, const FontSpec *val); /* Serialisation functions for Duplicate Session */ -int conf_serialised_size(Conf *conf); -void conf_serialise(Conf *conf, void *data); +void conf_serialise(BinarySink *bs, Conf *conf); int conf_deserialise(Conf *conf, void *data, int maxsize);/*returns size used*/ /* @@ -1028,7 +1028,7 @@ int conf_deserialise(Conf *conf, void *data, int maxsize);/*returns size used*/ */ FontSpec *fontspec_copy(const FontSpec *f); void fontspec_free(FontSpec *f); -int fontspec_serialise(FontSpec *f, void *data); +void fontspec_serialise(BinarySink *bs, FontSpec *f); FontSpec *fontspec_deserialise(void *data, int maxsize, int *used); /* @@ -1463,7 +1463,7 @@ int filename_equal(const Filename *f1, const Filename *f2); int filename_is_null(const Filename *fn); Filename *filename_copy(const Filename *fn); void filename_free(Filename *fn); -int filename_serialise(const Filename *f, void *data); +void filename_serialise(BinarySink *bs, const Filename *f); Filename *filename_deserialise(void *data, int maxsize, int *used); char *get_username(void); /* return value needs freeing */ char *get_random_data(int bytes, const char *device); /* used in cmdgen.c */ diff --git a/unix/gtkmain.c b/unix/gtkmain.c index 8136676e..ff6a15cf 100644 --- a/unix/gtkmain.c +++ b/unix/gtkmain.c @@ -156,8 +156,8 @@ void launch_duplicate_session(Conf *conf) * into a byte stream, create a pipe, and send this byte stream * to the child through the pipe. */ - int i, ret, sersize, size; - char *data; + int i, ret; + strbuf *serialised; char option[80]; int pipefd[2]; @@ -166,35 +166,27 @@ void launch_duplicate_session(Conf *conf) return; } - size = sersize = conf_serialised_size(conf); - if (use_pty_argv && pty_argv) { + serialised = strbuf_new(); + + conf_serialise(BinarySink_UPCAST(serialised), conf); + if (use_pty_argv && pty_argv) for (i = 0; pty_argv[i]; i++) - size += strlen(pty_argv[i]) + 1; - } + put_asciz(serialised, pty_argv[i]); - data = snewn(size, char); - conf_serialise(conf, data); - if (use_pty_argv && pty_argv) { - int p = sersize; - for (i = 0; pty_argv[i]; i++) { - strcpy(data + p, pty_argv[i]); - p += strlen(pty_argv[i]) + 1; - } - assert(p == size); - } - - sprintf(option, "---[%d,%d]", pipefd[0], size); + sprintf(option, "---[%d,%d]", pipefd[0], serialised->len); noncloexec(pipefd[0]); fork_and_exec_self(pipefd[1], option, NULL); close(pipefd[0]); i = ret = 0; - while (i < size && (ret = write(pipefd[1], data + i, size - i)) > 0) + while (i < serialised->len && + (ret = write(pipefd[1], serialised->s + i, + serialised->len - i)) > 0) i += ret; if (ret < 0) perror("write to pipe"); close(pipefd[1]); - sfree(data); + strbuf_free(serialised); } void launch_new_session(void) diff --git a/unix/uxmisc.c b/unix/uxmisc.c index 04af45d0..e96829c4 100644 --- a/unix/uxmisc.c +++ b/unix/uxmisc.c @@ -74,14 +74,9 @@ void filename_free(Filename *fn) sfree(fn); } -int filename_serialise(const Filename *f, void *vdata) +void filename_serialise(BinarySink *bs, const Filename *f) { - char *data = (char *)vdata; - int len = strlen(f->path) + 1; /* include trailing NUL */ - if (data) { - strcpy(data, f->path); - } - return len; + put_asciz(bs, f->path); } Filename *filename_deserialise(void *vdata, int maxsize, int *used) { @@ -275,12 +270,9 @@ void fontspec_free(FontSpec *f) sfree(f->name); sfree(f); } -int fontspec_serialise(FontSpec *f, void *data) +void fontspec_serialise(BinarySink *bs, FontSpec *f) { - int len = strlen(f->name); - if (data) - strcpy(data, f->name); - return len + 1; /* include trailing NUL */ + put_asciz(bs, f->name); } FontSpec *fontspec_deserialise(void *vdata, int maxsize, int *used) { diff --git a/windows/window.c b/windows/window.c index 5340f7d4..0a5d9c58 100644 --- a/windows/window.c +++ b/windows/window.c @@ -2107,10 +2107,13 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, * config structure. */ SECURITY_ATTRIBUTES sa; + strbuf *serbuf; void *p; int size; - size = conf_serialised_size(conf); + serbuf = strbuf_new(); + conf_serialise(BinarySink_UPCAST(serbuf), conf); + size = serbuf->len; sa.nLength = sizeof(sa); sa.lpSecurityDescriptor = NULL; @@ -2122,10 +2125,12 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, if (filemap && filemap != INVALID_HANDLE_VALUE) { p = MapViewOfFile(filemap, FILE_MAP_WRITE, 0, 0, size); if (p) { - conf_serialise(conf, p); + memcpy(p, serbuf->s, size); UnmapViewOfFile(p); } } + + strbuf_free(serbuf); inherit_handles = TRUE; cl = dupprintf("putty %s&%p:%u", argprefix, filemap, (unsigned)size); diff --git a/windows/winmisc.c b/windows/winmisc.c index e3fd861e..c3ec8306 100644 --- a/windows/winmisc.c +++ b/windows/winmisc.c @@ -50,14 +50,9 @@ void filename_free(Filename *fn) sfree(fn); } -int filename_serialise(const Filename *f, void *vdata) +void filename_serialise(BinarySink *bs, const Filename *f) { - char *data = (char *)vdata; - int len = strlen(f->path) + 1; /* include trailing NUL */ - if (data) { - strcpy(data, f->path); - } - return len; + put_asciz(bs, f->path); } Filename *filename_deserialise(void *vdata, int maxsize, int *used) { @@ -559,17 +554,12 @@ void fontspec_free(FontSpec *f) sfree(f->name); sfree(f); } -int fontspec_serialise(FontSpec *f, void *vdata) +void fontspec_serialise(BinarySink *bs, FontSpec *f) { - char *data = (char *)vdata; - int len = strlen(f->name) + 1; /* include trailing NUL */ - if (data) { - strcpy(data, f->name); - PUT_32BIT_MSB_FIRST(data + len, f->isbold); - PUT_32BIT_MSB_FIRST(data + len + 4, f->height); - PUT_32BIT_MSB_FIRST(data + len + 8, f->charset); - } - return len + 12; /* also include three 4-byte ints */ + put_asciz(bs, f->name); + put_uint32(bs, f->isbold); + put_uint32(bs, f->height); + put_uint32(bs, f->charset); } FontSpec *fontspec_deserialise(void *vdata, int maxsize, int *used) {