[tor-commits] [tor/master] Split reply formatting out of control_fmt.c

dgoulet at torproject.org dgoulet at torproject.org
Fri May 3 14:57:34 UTC 2019


commit 8e7316bae43e666a9fa3b99448561af90cbc21b8
Author: Taylor Yu <catalyst at torproject.org>
Date:   Mon Apr 8 11:34:12 2019 -0500

    Split reply formatting out of control_fmt.c
    
    Split the core reply formatting code out of control_fmt.c into
    control_proto.c.  The remaining code in control_format.c deals with
    specific subsystems and will eventually move to join those subsystems.
---
 scripts/maint/practracker/exceptions.txt |   2 +-
 src/core/include.am                      |   2 +
 src/feature/control/control.c            |   2 +-
 src/feature/control/control_auth.c       |   2 +-
 src/feature/control/control_cmd.c        |   2 +-
 src/feature/control/control_events.c     |   1 +
 src/feature/control/control_fmt.c        | 142 +-------------------------
 src/feature/control/control_fmt.h        |   9 --
 src/feature/control/control_getinfo.c    |   1 +
 src/feature/control/control_proto.c      | 165 +++++++++++++++++++++++++++++++
 src/feature/control/control_proto.h      |  24 +++++
 src/test/test_util.c                     |   2 +-
 12 files changed, 199 insertions(+), 155 deletions(-)

diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
index d90ed1f4b..ea774e2e8 100644
--- a/scripts/maint/practracker/exceptions.txt
+++ b/scripts/maint/practracker/exceptions.txt
@@ -152,7 +152,7 @@ problem function-size /src/feature/control/control_cmd.c:handle_control_add_onio
 problem function-size /src/feature/control/control_cmd.c:add_onion_helper_keyarg() 125
 problem function-size /src/feature/control/control_cmd.c:handle_control_command() 104
 problem function-size /src/feature/control/control_events.c:control_event_stream_status() 119
-problem include-count /src/feature/control/control_getinfo.c 53
+problem include-count /src/feature/control/control_getinfo.c 54
 problem function-size /src/feature/control/control_getinfo.c:getinfo_helper_misc() 109
 problem function-size /src/feature/control/control_getinfo.c:getinfo_helper_dir() 304
 problem function-size /src/feature/control/control_getinfo.c:getinfo_helper_events() 236
diff --git a/src/core/include.am b/src/core/include.am
index 4ec42182a..2533a4990 100644
--- a/src/core/include.am
+++ b/src/core/include.am
@@ -79,6 +79,7 @@ LIBTOR_APP_A_SOURCES = 				\
 	src/feature/control/control_events.c	\
 	src/feature/control/control_fmt.c	\
 	src/feature/control/control_getinfo.c	\
+	src/feature/control/control_proto.c	\
 	src/feature/control/fmt_serverstatus.c  \
 	src/feature/control/getinfo_geoip.c	\
 	src/feature/dirauth/keypin.c		\
@@ -307,6 +308,7 @@ noinst_HEADERS +=					\
 	src/feature/control/control_events.h	        \
 	src/feature/control/control_fmt.h		\
 	src/feature/control/control_getinfo.h		\
+	src/feature/control/control_proto.h		\
 	src/feature/control/fmt_serverstatus.h		\
 	src/feature/control/getinfo_geoip.h		\
 	src/feature/dirauth/authmode.h			\
diff --git a/src/feature/control/control.c b/src/feature/control/control.c
index 23ef83ef9..1f85bd3fc 100644
--- a/src/feature/control/control.c
+++ b/src/feature/control/control.c
@@ -47,7 +47,7 @@
 #include "feature/control/control_auth.h"
 #include "feature/control/control_cmd.h"
 #include "feature/control/control_events.h"
-#include "feature/control/control_fmt.h"
+#include "feature/control/control_proto.h"
 #include "feature/rend/rendcommon.h"
 #include "feature/rend/rendservice.h"
 #include "lib/evloop/procmon.h"
diff --git a/src/feature/control/control_auth.c b/src/feature/control/control_auth.c
index a86442c21..bdabdc989 100644
--- a/src/feature/control/control_auth.c
+++ b/src/feature/control/control_auth.c
@@ -15,7 +15,7 @@
 #include "feature/control/control_auth.h"
 #include "feature/control/control_cmd_args_st.h"
 #include "feature/control/control_connection_st.h"
-#include "feature/control/control_fmt.h"
+#include "feature/control/control_proto.h"
 #include "lib/crypt_ops/crypto_rand.h"
 #include "lib/crypt_ops/crypto_util.h"
 #include "lib/encoding/confline.h"
diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c
index 9afa734d8..d69177f4c 100644
--- a/src/feature/control/control_cmd.c
+++ b/src/feature/control/control_cmd.c
@@ -27,8 +27,8 @@
 #include "feature/control/control_auth.h"
 #include "feature/control/control_cmd.h"
 #include "feature/control/control_events.h"
-#include "feature/control/control_fmt.h"
 #include "feature/control/control_getinfo.h"
+#include "feature/control/control_proto.h"
 #include "feature/hs/hs_control.h"
 #include "feature/nodelist/nodelist.h"
 #include "feature/nodelist/routerinfo.h"
diff --git a/src/feature/control/control_events.c b/src/feature/control/control_events.c
index 129776f49..e596a8aee 100644
--- a/src/feature/control/control_events.c
+++ b/src/feature/control/control_events.c
@@ -24,6 +24,7 @@
 #include "feature/control/control.h"
 #include "feature/control/control_events.h"
 #include "feature/control/control_fmt.h"
+#include "feature/control/control_proto.h"
 #include "feature/dircommon/directory.h"
 #include "feature/nodelist/networkstatus.h"
 #include "feature/nodelist/nodelist.h"
diff --git a/src/feature/control/control_fmt.c b/src/feature/control/control_fmt.c
index dac3f8779..e0e77eb2d 100644
--- a/src/feature/control/control_fmt.c
+++ b/src/feature/control/control_fmt.c
@@ -14,6 +14,7 @@
 #include "core/or/circuitlist.h"
 #include "core/or/connection_edge.h"
 #include "feature/control/control_fmt.h"
+#include "feature/control/control_proto.h"
 #include "feature/nodelist/nodelist.h"
 
 #include "core/or/cpath_build_state_st.h"
@@ -23,39 +24,6 @@
 #include "core/or/socks_request_st.h"
 #include "feature/control/control_connection_st.h"
 
-/** Append a NUL-terminated string <b>s</b> to the end of
- * <b>conn</b>-\>outbuf.
- */
-void
-connection_write_str_to_buf(const char *s, control_connection_t *conn)
-{
-  size_t len = strlen(s);
-  connection_buf_add(s, len, TO_CONN(conn));
-}
-
-/** Acts like sprintf, but writes its formatted string to the end of
- * <b>conn</b>-\>outbuf. */
-void
-connection_printf_to_buf(control_connection_t *conn, const char *format, ...)
-{
-  va_list ap;
-  char *buf = NULL;
-  int len;
-
-  va_start(ap,format);
-  len = tor_vasprintf(&buf, format, ap);
-  va_end(ap);
-
-  if (len < 0) {
-    log_err(LD_BUG, "Unable to format string for controller.");
-    tor_assert(0);
-  }
-
-  connection_buf_add(buf, (size_t)len, TO_CONN(conn));
-
-  tor_free(buf);
-}
-
 /** Given an AP connection <b>conn</b> and a <b>len</b>-character buffer
  * <b>buf</b>, determine the address:port combination requested on
  * <b>conn</b>, and write it to <b>buf</b>.  Return 0 on success, -1 on
@@ -197,114 +165,6 @@ circuit_describe_status_for_controller(origin_circuit_t *circ)
   return rv;
 }
 
-/** Given a <b>len</b>-character string in <b>data</b>, made of lines
- * terminated by CRLF, allocate a new string in *<b>out</b>, and copy the
- * contents of <b>data</b> into *<b>out</b>, adding a period before any period
- * that appears at the start of a line, and adding a period-CRLF line at
- * the end. Replace all LF characters sequences with CRLF.  Return the number
- * of bytes in *<b>out</b>.
- */
-size_t
-write_escaped_data(const char *data, size_t len, char **out)
-{
-  tor_assert(len < SIZE_MAX - 9);
-  size_t sz_out = len+8+1;
-  char *outp;
-  const char *start = data, *end;
-  size_t i;
-  int start_of_line;
-  for (i=0; i < len; ++i) {
-    if (data[i] == '\n') {
-      sz_out += 2; /* Maybe add a CR; maybe add a dot. */
-      if (sz_out >= SIZE_T_CEILING) {
-        log_warn(LD_BUG, "Input to write_escaped_data was too long");
-        *out = tor_strdup(".\r\n");
-        return 3;
-      }
-    }
-  }
-  *out = outp = tor_malloc(sz_out);
-  end = data+len;
-  start_of_line = 1;
-  while (data < end) {
-    if (*data == '\n') {
-      if (data > start && data[-1] != '\r')
-        *outp++ = '\r';
-      start_of_line = 1;
-    } else if (*data == '.') {
-      if (start_of_line) {
-        start_of_line = 0;
-        *outp++ = '.';
-      }
-    } else {
-      start_of_line = 0;
-    }
-    *outp++ = *data++;
-  }
-  if (outp < *out+2 || fast_memcmp(outp-2, "\r\n", 2)) {
-    *outp++ = '\r';
-    *outp++ = '\n';
-  }
-  *outp++ = '.';
-  *outp++ = '\r';
-  *outp++ = '\n';
-  *outp = '\0'; /* NUL-terminate just in case. */
-  tor_assert(outp >= *out);
-  tor_assert((size_t)(outp - *out) <= sz_out);
-  return outp - *out;
-}
-
-/** Given a <b>len</b>-character string in <b>data</b>, made of lines
- * terminated by CRLF, allocate a new string in *<b>out</b>, and copy
- * the contents of <b>data</b> into *<b>out</b>, removing any period
- * that appears at the start of a line, and replacing all CRLF sequences
- * with LF.   Return the number of
- * bytes in *<b>out</b>. */
-size_t
-read_escaped_data(const char *data, size_t len, char **out)
-{
-  char *outp;
-  const char *next;
-  const char *end;
-
-  *out = outp = tor_malloc(len+1);
-
-  end = data+len;
-
-  while (data < end) {
-    /* we're at the start of a line. */
-    if (*data == '.')
-      ++data;
-    next = memchr(data, '\n', end-data);
-    if (next) {
-      size_t n_to_copy = next-data;
-      /* Don't copy a CR that precedes this LF. */
-      if (n_to_copy && *(next-1) == '\r')
-        --n_to_copy;
-      memcpy(outp, data, n_to_copy);
-      outp += n_to_copy;
-      data = next+1; /* This will point at the start of the next line,
-                      * or the end of the string, or a period. */
-    } else {
-      memcpy(outp, data, end-data);
-      outp += (end-data);
-      *outp = '\0';
-      return outp - *out;
-    }
-    *outp++ = '\n';
-  }
-
-  *outp = '\0';
-  return outp - *out;
-}
-
-/** Send a "DONE" message down the control connection <b>conn</b>. */
-void
-send_control_done(control_connection_t *conn)
-{
-  connection_write_str_to_buf("250 OK\r\n", conn);
-}
-
 /** Return a longname the node whose identity is <b>id_digest</b>. If
  * node_get_by_id() returns NULL, base 16 encoding of <b>id_digest</b> is
  * returned instead.
diff --git a/src/feature/control/control_fmt.h b/src/feature/control/control_fmt.h
index 8bbbaa95d..6446e3707 100644
--- a/src/feature/control/control_fmt.h
+++ b/src/feature/control/control_fmt.h
@@ -12,21 +12,12 @@
 #ifndef TOR_CONTROL_FMT_H
 #define TOR_CONTROL_FMT_H
 
-void connection_write_str_to_buf(const char *s, control_connection_t *conn);
-void connection_printf_to_buf(control_connection_t *conn,
-                                     const char *format, ...)
-  CHECK_PRINTF(2,3);
-
 int write_stream_target_to_buf(entry_connection_t *conn, char *buf,
                                size_t len);
 void orconn_target_get_name(char *buf, size_t len,
                             or_connection_t *conn);
 char *circuit_describe_status_for_controller(origin_circuit_t *circ);
 
-size_t write_escaped_data(const char *data, size_t len, char **out);
-size_t read_escaped_data(const char *data, size_t len, char **out);
-void send_control_done(control_connection_t *conn);
-
 MOCK_DECL(const char *, node_describe_longname_by_id,(const char *id_digest));
 
 #endif /* !defined(TOR_CONTROL_FMT_H) */
diff --git a/src/feature/control/control_getinfo.c b/src/feature/control/control_getinfo.c
index 5c6a0d4aa..6f5e69dc0 100644
--- a/src/feature/control/control_getinfo.c
+++ b/src/feature/control/control_getinfo.c
@@ -28,6 +28,7 @@
 #include "feature/control/control_events.h"
 #include "feature/control/control_fmt.h"
 #include "feature/control/control_getinfo.h"
+#include "feature/control/control_proto.h"
 #include "feature/control/fmt_serverstatus.h"
 #include "feature/control/getinfo_geoip.h"
 #include "feature/dircache/dirserv.h"
diff --git a/src/feature/control/control_proto.c b/src/feature/control/control_proto.c
new file mode 100644
index 000000000..daf5b1189
--- /dev/null
+++ b/src/feature/control/control_proto.c
@@ -0,0 +1,165 @@
+/* Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * \file control_proto.c
+ * \brief Formatting functions for controller data.
+ */
+
+#include "core/or/or.h"
+
+#include "core/mainloop/connection.h"
+#include "core/or/circuitbuild.h"
+#include "core/or/circuitlist.h"
+#include "core/or/connection_edge.h"
+#include "feature/control/control_proto.h"
+#include "feature/nodelist/nodelist.h"
+
+#include "core/or/cpath_build_state_st.h"
+#include "core/or/entry_connection_st.h"
+#include "core/or/or_connection_st.h"
+#include "core/or/origin_circuit_st.h"
+#include "core/or/socks_request_st.h"
+#include "feature/control/control_connection_st.h"
+
+/** Append a NUL-terminated string <b>s</b> to the end of
+ * <b>conn</b>-\>outbuf.
+ */
+void
+connection_write_str_to_buf(const char *s, control_connection_t *conn)
+{
+  size_t len = strlen(s);
+  connection_buf_add(s, len, TO_CONN(conn));
+}
+
+/** Acts like sprintf, but writes its formatted string to the end of
+ * <b>conn</b>-\>outbuf. */
+void
+connection_printf_to_buf(control_connection_t *conn, const char *format, ...)
+{
+  va_list ap;
+  char *buf = NULL;
+  int len;
+
+  va_start(ap,format);
+  len = tor_vasprintf(&buf, format, ap);
+  va_end(ap);
+
+  if (len < 0) {
+    log_err(LD_BUG, "Unable to format string for controller.");
+    tor_assert(0);
+  }
+
+  connection_buf_add(buf, (size_t)len, TO_CONN(conn));
+
+  tor_free(buf);
+}
+
+/** Given a <b>len</b>-character string in <b>data</b>, made of lines
+ * terminated by CRLF, allocate a new string in *<b>out</b>, and copy the
+ * contents of <b>data</b> into *<b>out</b>, adding a period before any period
+ * that appears at the start of a line, and adding a period-CRLF line at
+ * the end. Replace all LF characters sequences with CRLF.  Return the number
+ * of bytes in *<b>out</b>.
+ */
+size_t
+write_escaped_data(const char *data, size_t len, char **out)
+{
+  tor_assert(len < SIZE_MAX - 9);
+  size_t sz_out = len+8+1;
+  char *outp;
+  const char *start = data, *end;
+  size_t i;
+  int start_of_line;
+  for (i=0; i < len; ++i) {
+    if (data[i] == '\n') {
+      sz_out += 2; /* Maybe add a CR; maybe add a dot. */
+      if (sz_out >= SIZE_T_CEILING) {
+        log_warn(LD_BUG, "Input to write_escaped_data was too long");
+        *out = tor_strdup(".\r\n");
+        return 3;
+      }
+    }
+  }
+  *out = outp = tor_malloc(sz_out);
+  end = data+len;
+  start_of_line = 1;
+  while (data < end) {
+    if (*data == '\n') {
+      if (data > start && data[-1] != '\r')
+        *outp++ = '\r';
+      start_of_line = 1;
+    } else if (*data == '.') {
+      if (start_of_line) {
+        start_of_line = 0;
+        *outp++ = '.';
+      }
+    } else {
+      start_of_line = 0;
+    }
+    *outp++ = *data++;
+  }
+  if (outp < *out+2 || fast_memcmp(outp-2, "\r\n", 2)) {
+    *outp++ = '\r';
+    *outp++ = '\n';
+  }
+  *outp++ = '.';
+  *outp++ = '\r';
+  *outp++ = '\n';
+  *outp = '\0'; /* NUL-terminate just in case. */
+  tor_assert(outp >= *out);
+  tor_assert((size_t)(outp - *out) <= sz_out);
+  return outp - *out;
+}
+
+/** Given a <b>len</b>-character string in <b>data</b>, made of lines
+ * terminated by CRLF, allocate a new string in *<b>out</b>, and copy
+ * the contents of <b>data</b> into *<b>out</b>, removing any period
+ * that appears at the start of a line, and replacing all CRLF sequences
+ * with LF.   Return the number of
+ * bytes in *<b>out</b>. */
+size_t
+read_escaped_data(const char *data, size_t len, char **out)
+{
+  char *outp;
+  const char *next;
+  const char *end;
+
+  *out = outp = tor_malloc(len+1);
+
+  end = data+len;
+
+  while (data < end) {
+    /* we're at the start of a line. */
+    if (*data == '.')
+      ++data;
+    next = memchr(data, '\n', end-data);
+    if (next) {
+      size_t n_to_copy = next-data;
+      /* Don't copy a CR that precedes this LF. */
+      if (n_to_copy && *(next-1) == '\r')
+        --n_to_copy;
+      memcpy(outp, data, n_to_copy);
+      outp += n_to_copy;
+      data = next+1; /* This will point at the start of the next line,
+                      * or the end of the string, or a period. */
+    } else {
+      memcpy(outp, data, end-data);
+      outp += (end-data);
+      *outp = '\0';
+      return outp - *out;
+    }
+    *outp++ = '\n';
+  }
+
+  *outp = '\0';
+  return outp - *out;
+}
+
+/** Send a "DONE" message down the control connection <b>conn</b>. */
+void
+send_control_done(control_connection_t *conn)
+{
+  connection_write_str_to_buf("250 OK\r\n", conn);
+}
diff --git a/src/feature/control/control_proto.h b/src/feature/control/control_proto.h
new file mode 100644
index 000000000..5720b2260
--- /dev/null
+++ b/src/feature/control/control_proto.h
@@ -0,0 +1,24 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * \file control_proto.h
+ * \brief Header file for control_proto.c.
+ **/
+
+#ifndef TOR_CONTROL_PROTO_H
+#define TOR_CONTROL_PROTO_H
+
+void connection_write_str_to_buf(const char *s, control_connection_t *conn);
+void connection_printf_to_buf(control_connection_t *conn,
+                                     const char *format, ...)
+  CHECK_PRINTF(2,3);
+
+size_t write_escaped_data(const char *data, size_t len, char **out);
+size_t read_escaped_data(const char *data, size_t len, char **out);
+void send_control_done(control_connection_t *conn);
+
+#endif /* !defined(TOR_CONTROL_PROTO_H) */
diff --git a/src/test/test_util.c b/src/test/test_util.c
index 61e41e9a9..88eda847a 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -15,7 +15,7 @@
 #include "lib/buf/buffers.h"
 #include "app/config/config.h"
 #include "feature/control/control.h"
-#include "feature/control/control_fmt.h"
+#include "feature/control/control_proto.h"
 #include "feature/client/transports.h"
 #include "lib/crypt_ops/crypto_format.h"
 #include "lib/crypt_ops/crypto_rand.h"





More information about the tor-commits mailing list