From 09a3c949f8388c83d4b62e281474687a26596a59 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Sat, 9 Feb 2019 16:06:32 +0200 Subject: [PATCH 1/3] Add connection_dir_buf_add() helper function --- src/core/mainloop/connection.c | 17 +++++++++++++++++ src/core/mainloop/connection.h | 2 ++ 2 files changed, 19 insertions(+) diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index 37f35c8b8d..b53552a53d 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -4341,6 +4341,23 @@ connection_write_to_buf_impl_,(const char *string, size_t len, connection_write_to_buf_commit(conn, written); } +/** + * Write a string (of size len to directory connection + * dir_conn. Apply compression if connection is configured to use + * it and finalize it if done is true. + */ +void +connection_dir_buf_add(const char *string, size_t len, + dir_connection_t *dir_conn, int done) +{ + if (dir_conn->compress_state != NULL) { + connection_buf_add_compress(string, len, dir_conn, done); + return; + } + + connection_buf_add(string, len, TO_CONN(dir_conn)); +} + void connection_buf_add_compress(const char *string, size_t len, dir_connection_t *conn, int done) diff --git a/src/core/mainloop/connection.h b/src/core/mainloop/connection.h index f4f0e839ae..de6473251d 100644 --- a/src/core/mainloop/connection.h +++ b/src/core/mainloop/connection.h @@ -226,6 +226,8 @@ MOCK_DECL(void, connection_write_to_buf_impl_, /* DOCDOC connection_write_to_buf */ static void connection_buf_add(const char *string, size_t len, connection_t *conn); +void connection_dir_buf_add(const char *string, size_t len, + dir_connection_t *dir_conn, int done); static inline void connection_buf_add(const char *string, size_t len, connection_t *conn) { From 4c102213326705079be60d3ce335b81c94b76499 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Sat, 9 Feb 2019 16:46:31 +0200 Subject: [PATCH 2/3] Use compress_dir_buf_add() function in a few places --- src/feature/dircache/dircache.c | 28 ++++++++++------------------ src/feature/dircache/dirserv.c | 20 +++++++------------- 2 files changed, 17 insertions(+), 31 deletions(-) diff --git a/src/feature/dircache/dircache.c b/src/feature/dircache/dircache.c index f6e57c5064..ee6e4f7a81 100644 --- a/src/feature/dircache/dircache.c +++ b/src/feature/dircache/dircache.c @@ -1068,13 +1068,11 @@ handle_get_status_vote(dir_connection_t *conn, const get_handler_args_t *args) if (compress_method != NO_METHOD) { conn->compress_state = tor_compress_new(1, compress_method, choose_compression_level(estimated_len)); - SMARTLIST_FOREACH(items, const char *, c, - connection_buf_add_compress(c, strlen(c), conn, 0)); - connection_buf_add_compress("", 0, conn, 1); - } else { - SMARTLIST_FOREACH(items, const char *, c, - connection_buf_add(c, strlen(c), TO_CONN(conn))); } + + SMARTLIST_FOREACH(items, const char *, c, + connection_dir_buf_add(c, strlen(c), conn, + c_sl_idx == c_sl_len - 1)); } else { SMARTLIST_FOREACH(dir_items, cached_dir_t *, d, connection_buf_add(compress_method != NO_METHOD ? @@ -1325,19 +1323,13 @@ handle_get_keys(dir_connection_t *conn, const get_handler_args_t *args) if (compress_method != NO_METHOD) { conn->compress_state = tor_compress_new(1, compress_method, choose_compression_level(len)); - SMARTLIST_FOREACH(certs, authority_cert_t *, c, - connection_buf_add_compress( - c->cache_info.signed_descriptor_body, - c->cache_info.signed_descriptor_len, - conn, 0)); - connection_buf_add_compress("", 0, conn, 1); - } else { - SMARTLIST_FOREACH(certs, authority_cert_t *, c, - connection_buf_add(c->cache_info.signed_descriptor_body, - c->cache_info.signed_descriptor_len, - TO_CONN(conn))); } - keys_done: + + SMARTLIST_FOREACH(certs, authority_cert_t *, c, + connection_dir_buf_add(c->cache_info.signed_descriptor_body, + c->cache_info.signed_descriptor_len, + conn, c_sl_idx == c_sl_len - 1)); + keys_done: smartlist_free(certs); goto done; } diff --git a/src/feature/dircache/dirserv.c b/src/feature/dircache/dirserv.c index 4be6836fe1..79400bf15f 100644 --- a/src/feature/dircache/dirserv.c +++ b/src/feature/dircache/dirserv.c @@ -583,11 +583,9 @@ spooled_resource_flush_some(spooled_resource_t *spooled, /* Absent objects count as "done". */ return SRFS_DONE; } - if (conn->compress_state) { - connection_buf_add_compress((const char*)body, bodylen, conn, 0); - } else { - connection_buf_add((const char*)body, bodylen, TO_CONN(conn)); - } + + connection_dir_buf_add((const char*)body, bodylen, conn, 0); + return SRFS_DONE; } else { cached_dir_t *cached = spooled->cached_dir_ref; @@ -622,14 +620,10 @@ spooled_resource_flush_some(spooled_resource_t *spooled, if (BUG(remaining < 0)) return SRFS_ERR; ssize_t bytes = (ssize_t) MIN(DIRSERV_CACHED_DIR_CHUNK_SIZE, remaining); - if (conn->compress_state) { - connection_buf_add_compress( - ptr + spooled->cached_dir_offset, - bytes, conn, 0); - } else { - connection_buf_add(ptr + spooled->cached_dir_offset, - bytes, TO_CONN(conn)); - } + + connection_dir_buf_add(ptr + spooled->cached_dir_offset, + bytes, conn, 0); + spooled->cached_dir_offset += bytes; if (spooled->cached_dir_offset >= (off_t)total_len) { return SRFS_DONE; From 8d04dc416bd6aa3fefc0914add8708083dc99d52 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Sat, 9 Feb 2019 16:56:54 +0200 Subject: [PATCH 3/3] Add changes file --- changes/ticket28816 | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changes/ticket28816 diff --git a/changes/ticket28816 b/changes/ticket28816 new file mode 100644 index 0000000000..02878ccfdc --- /dev/null +++ b/changes/ticket28816 @@ -0,0 +1,4 @@ + o Code simplification and refactoring: + - Introduce a connection_dir_buf_add() helper function that checks for + compress_state of dir_connection_t and automatically writes a string to + directory connection with or without compression. Resolves issue 28816.