Adding gpr_dump and gpr_hexdump as discussed. Removed gpr_slice_to_cstring as well.

13 files changed, 110 insertions, 109 deletions

diff --git a/include/grpc/support/slice.h b/include/grpc/support/slice.h
index a4fb2d0807..ec6c117afe 100644
--- a/include/grpc/support/slice.h
+++ b/include/grpc/support/slice.h
@@ -172,10 +172,6 @@ gpr_slice gpr_empty_slice(void);
 int gpr_slice_cmp(gpr_slice a, gpr_slice b);
 int gpr_slice_str_cmp(gpr_slice a, const char *b);
 
-/* Returns a null terminated C string from a slice. It is the responsibility of
-   the caller to call gpr_free on the result. */
-char *gpr_slice_to_cstring(gpr_slice s);
-
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/core/iomgr/tcp_posix.c b/src/core/iomgr/tcp_posix.c
index 9ad089af66..b6d6efc9fb 100644
--- a/src/core/iomgr/tcp_posix.c
+++ b/src/core/iomgr/tcp_posix.c
@@ -313,9 +313,7 @@ static void call_read_cb(grpc_tcp *tcp, gpr_slice *slices, size_t nslices,
     size_t i;
     gpr_log(GPR_DEBUG, "read: status=%d", status);
     for (i = 0; i < nslices; i++) {
-      char *dump =
-          gpr_hexdump((char *)GPR_SLICE_START_PTR(slices[i]),
-                      GPR_SLICE_LENGTH(slices[i]), GPR_HEXDUMP_PLAINTEXT);
+      char *dump = gpr_dump_slice(slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII);
       gpr_log(GPR_DEBUG, "READ: %s", dump);
       gpr_free(dump);
     }
@@ -540,9 +538,7 @@ static grpc_endpoint_write_status grpc_tcp_write(grpc_endpoint *ep,
     size_t i;
 
     for (i = 0; i < nslices; i++) {
-      char *data =
-          gpr_hexdump((char *)GPR_SLICE_START_PTR(slices[i]),
-                      GPR_SLICE_LENGTH(slices[i]), GPR_HEXDUMP_PLAINTEXT);
+      char *data = gpr_dump_slice(slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII);
       gpr_log(GPR_DEBUG, "WRITE %p: %s", tcp, data);
       gpr_free(data);
     }
diff --git a/src/core/security/secure_endpoint.c b/src/core/security/secure_endpoint.c
index 73496d1153..3548198046 100644
--- a/src/core/security/secure_endpoint.c
+++ b/src/core/security/secure_endpoint.c
@@ -101,9 +101,7 @@ static void call_read_cb(secure_endpoint *ep, gpr_slice *slices, size_t nslices,
   if (grpc_trace_secure_endpoint) {
     size_t i;
     for (i = 0; i < nslices; i++) {
-      char *data =
-          gpr_hexdump((char *)GPR_SLICE_START_PTR(slices[i]),
-                      GPR_SLICE_LENGTH(slices[i]), GPR_HEXDUMP_PLAINTEXT);
+      char *data = gpr_dump_slice(slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII);
       gpr_log(GPR_DEBUG, "READ %p: %s", ep, data);
       gpr_free(data);
     }
@@ -235,9 +233,7 @@ static grpc_endpoint_write_status endpoint_write(grpc_endpoint *secure_ep,
 
   if (grpc_trace_secure_endpoint) {
     for (i = 0; i < nslices; i++) {
-      char *data =
-          gpr_hexdump((char *)GPR_SLICE_START_PTR(slices[i]),
-                      GPR_SLICE_LENGTH(slices[i]), GPR_HEXDUMP_PLAINTEXT);
+      char *data = gpr_dump_slice(slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII);
       gpr_log(GPR_DEBUG, "WRITE %p: %s", ep, data);
       gpr_free(data);
     }
diff --git a/src/core/support/string.c b/src/core/support/string.c
index 6a80ccc841..09598da946 100644
--- a/src/core/support/string.c
+++ b/src/core/support/string.c
@@ -61,14 +61,14 @@ typedef struct {
   size_t capacity;
   size_t length;
   char *data;
-} hexout;
+} dump_out;
 
-static hexout hexout_create(void) {
-  hexout r = {0, 0, NULL};
+static dump_out dump_out_create(void) {
+  dump_out r = {0, 0, NULL};
   return r;
 }
 
-static void hexout_append(hexout *out, char c) {
+static void dump_out_append(dump_out *out, char c) {
   if (out->length == out->capacity) {
     out->capacity = GPR_MAX(8, 2 * out->capacity);
     out->data = gpr_realloc(out->data, out->capacity);
@@ -76,34 +76,55 @@ static void hexout_append(hexout *out, char c) {
   out->data[out->length++] = c;
 }
 
-char *gpr_hexdump(const char *buf, size_t len, gpr_uint32 flags) {
+static void hexdump(dump_out *out, const char *buf, size_t len) {
   static const char hex[16] = "0123456789abcdef";
-  hexout out = hexout_create();
 
   const gpr_uint8 *const beg = (const gpr_uint8 *)buf;
   const gpr_uint8 *const end = beg + len;
   const gpr_uint8 *cur;
 
   for (cur = beg; cur != end; ++cur) {
-    if (cur != beg) hexout_append(&out, ' ');
-    hexout_append(&out, hex[*cur >> 4]);
-    hexout_append(&out, hex[*cur & 0xf]);
+    if (cur != beg) dump_out_append(out, ' ');
+    dump_out_append(out, hex[*cur >> 4]);
+    dump_out_append(out, hex[*cur & 0xf]);
   }
+}
 
-  if (flags & GPR_HEXDUMP_PLAINTEXT) {
-    if (len) hexout_append(&out, ' ');
-    hexout_append(&out, '\'');
-    for (cur = beg; cur != end; ++cur) {
-      hexout_append(&out, isprint(*cur) ? *(char*)cur : '.');
-    }
-    hexout_append(&out, '\'');
+static void asciidump(dump_out *out, const char *buf, size_t len) {
+  const gpr_uint8 *const beg = (const gpr_uint8 *)buf;
+  const gpr_uint8 *const end = beg + len;
+  const gpr_uint8 *cur;
+  int out_was_empty = (out->length == 0);
+  if (!out_was_empty) {
+    dump_out_append(out, ' ');
+    dump_out_append(out, '\'');
   }
+  for (cur = beg; cur != end; ++cur) {
+    dump_out_append(out, isprint(*cur) ? *(char *)cur : '.');
+  }
+  if (!out_was_empty) {
+    dump_out_append(out, '\'');
+  }
+}
 
-  hexout_append(&out, 0);
-
+char *gpr_dump(const char *buf, size_t len, gpr_uint32 flags) {
+  dump_out out = dump_out_create();
+  if (flags & GPR_DUMP_HEX) {
+    hexdump(&out, buf, len);
+  }
+  if (flags & GPR_DUMP_ASCII) {
+    asciidump(&out, buf, len);
+  }
+  dump_out_append(&out, 0);
   return out.data;
 }
 
+char *gpr_dump_slice(gpr_slice s, gpr_uint32 flags) {
+  return gpr_dump((const char *)GPR_SLICE_START_PTR(s), GPR_SLICE_LENGTH(s),
+                  flags);
+}
+
+
 int gpr_parse_bytes_to_uint32(const char *buf, size_t len, gpr_uint32 *result) {
   gpr_uint32 out = 0;
   gpr_uint32 new;
diff --git a/src/core/support/string.h b/src/core/support/string.h
index 31e9fcb5e9..d950d908d6 100644
--- a/src/core/support/string.h
+++ b/src/core/support/string.h
@@ -37,6 +37,7 @@
 #include <stddef.h>
 
 #include <grpc/support/port_platform.h>
+#include <grpc/support/slice.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -44,12 +45,16 @@ extern "C" {
 
 /* String utility functions */
 
-/* flag to include plaintext after a hexdump */
-#define GPR_HEXDUMP_PLAINTEXT 0x00000001
+/* Flags for gpr_dump function. */
+#define GPR_DUMP_HEX   0x00000001
+#define GPR_DUMP_ASCII 0x00000002
 
-/* Converts array buf, of length len, into a hexadecimal dump. Result should
-   be freed with gpr_free() */
-char *gpr_hexdump(const char *buf, size_t len, gpr_uint32 flags);
+/* Converts array buf, of length len, into a C string  according to the flags.
+   Result should be freed with gpr_free() */
+char *gpr_dump(const char *buf, size_t len, gpr_uint32 flags);
+
+/* Calls gpr_dump on a slice. */
+char *gpr_dump_slice(gpr_slice slice, gpr_uint32 flags);
 
 /* Parses an array of bytes into an integer (base 10). Returns 1 on success,
    0 on failure. */
diff --git a/src/core/surface/call_log_batch.c b/src/core/surface/call_log_batch.c
index 55663298c9..997046d954 100644
--- a/src/core/surface/call_log_batch.c
+++ b/src/core/surface/call_log_batch.c
@@ -46,8 +46,8 @@ static void add_metadata(gpr_strvec *b, const grpc_metadata *md, size_t count) {
     gpr_strvec_add(b, gpr_strdup(md[i].key));
 
     gpr_strvec_add(b, gpr_strdup(" value="));
-    gpr_strvec_add(b, gpr_hexdump(md[i].value, md[i].value_length,
-                                  GPR_HEXDUMP_PLAINTEXT));
+    gpr_strvec_add(b, gpr_dump(md[i].value, md[i].value_length,
+                               GPR_DUMP_HEX | GPR_DUMP_ASCII));
   }
 }
 
diff --git a/src/core/transport/chttp2/hpack_parser.c b/src/core/transport/chttp2/hpack_parser.c
index a489543868..d164d0720f 100644
--- a/src/core/transport/chttp2/hpack_parser.c
+++ b/src/core/transport/chttp2/hpack_parser.c
@@ -1320,12 +1320,9 @@ static int parse_value_string_with_literal_key(grpc_chttp2_hpack_parser *p,
 /* PUBLIC INTERFACE */
 
 static void on_header_not_set(void *user_data, grpc_mdelem *md) {
-  char *keyhex =
-      gpr_hexdump(grpc_mdstr_as_c_string(md->key),
-                  GPR_SLICE_LENGTH(md->key->slice), GPR_HEXDUMP_PLAINTEXT);
+  char *keyhex = gpr_dump_slice(md->key->slice, GPR_DUMP_HEX | GPR_DUMP_ASCII);
   char *valuehex =
-      gpr_hexdump(grpc_mdstr_as_c_string(md->value),
-                  GPR_SLICE_LENGTH(md->value->slice), GPR_HEXDUMP_PLAINTEXT);
+      gpr_dump_slice(md->value->slice, GPR_DUMP_HEX | GPR_DUMP_ASCII);
   gpr_log(GPR_ERROR, "on_header callback not set; key=%s value=%s", keyhex,
           valuehex);
   gpr_free(keyhex);
diff --git a/src/core/transport/transport_op_string.c b/src/core/transport/transport_op_string.c
index 5c4edb006a..8d633d1ba2 100644
--- a/src/core/transport/transport_op_string.c
+++ b/src/core/transport/transport_op_string.c
@@ -47,14 +47,12 @@
 
 static void put_metadata(gpr_strvec *b, grpc_mdelem *md) {
   gpr_strvec_add(b, gpr_strdup("key="));
-  gpr_strvec_add(
-      b, gpr_hexdump((char *)GPR_SLICE_START_PTR(md->key->slice),
-                     GPR_SLICE_LENGTH(md->key->slice), GPR_HEXDUMP_PLAINTEXT));
+  gpr_strvec_add(b,
+                 gpr_dump_slice(md->key->slice, GPR_DUMP_HEX | GPR_DUMP_ASCII));
 
   gpr_strvec_add(b, gpr_strdup(" value="));
-  gpr_strvec_add(b, gpr_hexdump((char *)GPR_SLICE_START_PTR(md->value->slice),
-                                GPR_SLICE_LENGTH(md->value->slice),
-                                GPR_HEXDUMP_PLAINTEXT));
+  gpr_strvec_add(
+      b, gpr_dump_slice(md->value->slice, GPR_DUMP_HEX | GPR_DUMP_ASCII));
 }
 
 static void put_metadata_list(gpr_strvec *b, grpc_metadata_batch md) {
diff --git a/test/core/bad_client/bad_client.c b/test/core/bad_client/bad_client.c
index e9adcf34c7..2590d4dc4b 100644
--- a/test/core/bad_client/bad_client.c
+++ b/test/core/bad_client/bad_client.c
@@ -83,8 +83,8 @@ void grpc_run_bad_client_test(grpc_bad_client_server_side_validator validator,
   gpr_slice slice =
       gpr_slice_from_copied_buffer(client_payload, client_payload_length);
 
-  hex =
-      gpr_hexdump(client_payload, client_payload_length, GPR_HEXDUMP_PLAINTEXT);
+  hex = gpr_dump(client_payload, client_payload_length,
+                 GPR_DUMP_HEX | GPR_DUMP_ASCII);
 
   /* Add a debug log */
   gpr_log(GPR_INFO, "TEST: %s", hex);
diff --git a/test/core/support/slice_test.c b/test/core/support/slice_test.c
index 3f6522f8bf..3ca87427dd 100644
--- a/test/core/support/slice_test.c
+++ b/test/core/support/slice_test.c
@@ -212,33 +212,6 @@ static void test_slice_from_copied_string_works(void) {
   gpr_slice_unref(slice);
 }
 
-static void test_slice_to_cstring_works(void) {
-  static const char *text = "HELLO WORLD!";
-  static const char *long_text =
-      "It was a bright cold day in April, and the clocks were striking "
-      "thirteen. Winston Smith, his chin nuzzled into his breast in an effort "
-      "to escape the vile wind, slipped quickly through the glass doors of "
-      "Victory Mansions, though not quickly enough to prevent a swirl of "
-      "gritty dust from entering along with him.";
-  gpr_slice slice;
-  char *text2;
-  char *long_text2;
-
-  LOG_TEST_NAME("test_slice_to_cstring_works");
-
-  slice = gpr_slice_from_copied_string(text);
-  text2 = gpr_slice_to_cstring(slice);
-  GPR_ASSERT(strcmp(text, text2) == 0);
-  gpr_free(text2);
-  gpr_slice_unref(slice);
-
-  slice = gpr_slice_from_copied_string(long_text);
-  long_text2 = gpr_slice_to_cstring(slice);
-  GPR_ASSERT(strcmp(long_text, long_text2) == 0);
-  gpr_free(long_text2);
-  gpr_slice_unref(slice);
-}
-
 int main(int argc, char **argv) {
   unsigned length;
   grpc_test_init(argc, argv);
@@ -251,6 +224,5 @@ int main(int argc, char **argv) {
     test_slice_split_tail_works(length);
   }
   test_slice_from_copied_string_works();
-  test_slice_to_cstring_works();
   return 0;
 }
diff --git a/test/core/support/string_test.c b/test/core/support/string_test.c
index b59082eecf..f04e72ac2b 100644
--- a/test/core/support/string_test.c
+++ b/test/core/support/string_test.c
@@ -58,21 +58,49 @@ static void test_strdup(void) {
   GPR_ASSERT(NULL == gpr_strdup(NULL));
 }
 
-static void expect_hexdump(const char *buf, size_t len, gpr_uint32 flags,
-                           const char *result) {
-  char *got = gpr_hexdump(buf, len, flags);
+static void expect_dump(const char *buf, size_t len, gpr_uint32 flags,
+                        const char *result) {
+  char *got = gpr_dump(buf, len, flags);
   GPR_ASSERT(0 == strcmp(got, result));
   gpr_free(got);
 }
 
-static void test_hexdump(void) {
-  LOG_TEST_NAME("test_hexdump");
-  expect_hexdump("\x01", 1, 0, "01");
-  expect_hexdump("\x01", 1, GPR_HEXDUMP_PLAINTEXT, "01 '.'");
-  expect_hexdump("\x01\x02", 2, 0, "01 02");
-  expect_hexdump("\x01\x23\x45\x67\x89\xab\xcd\xef", 8, 0,
+static void test_dump(void) {
+  LOG_TEST_NAME("test_dump");
+  expect_dump("\x01", 1, GPR_DUMP_HEX, "01");
+  expect_dump("\x01", 1, GPR_DUMP_HEX | GPR_DUMP_ASCII, "01 '.'");
+  expect_dump("\x01\x02", 2, GPR_DUMP_HEX, "01 02");
+  expect_dump("\x01\x23\x45\x67\x89\xab\xcd\xef", 8, GPR_DUMP_HEX,
                  "01 23 45 67 89 ab cd ef");
-  expect_hexdump("ab", 2, GPR_HEXDUMP_PLAINTEXT, "61 62 'ab'");
+  expect_dump("ab", 2, GPR_DUMP_HEX | GPR_DUMP_ASCII, "61 62 'ab'");
+}
+
+static void expect_slice_dump(gpr_slice slice, gpr_uint32 flags,
+                              const char *result) {
+  char *got = gpr_dump_slice(slice, flags);
+  GPR_ASSERT(0 == strcmp(got, result));
+  gpr_free(got);
+  gpr_slice_unref(slice);
+}
+
+static void test_dump_slice(void) {
+  static const char *text = "HELLO WORLD!";
+  static const char *long_text =
+      "It was a bright cold day in April, and the clocks were striking "
+      "thirteen. Winston Smith, his chin nuzzled into his breast in an effort "
+      "to escape the vile wind, slipped quickly through the glass doors of "
+      "Victory Mansions, though not quickly enough to prevent a swirl of "
+      "gritty dust from entering along with him.";
+
+  LOG_TEST_NAME("test_dump_slice");
+
+  expect_slice_dump(gpr_slice_from_copied_string(text), GPR_DUMP_ASCII, text);
+  expect_slice_dump(gpr_slice_from_copied_string(long_text), GPR_DUMP_ASCII,
+                    long_text);
+  expect_slice_dump(gpr_slice_from_copied_buffer("\x01", 1), GPR_DUMP_HEX,
+                    "01");
+  expect_slice_dump(gpr_slice_from_copied_buffer("\x01", 1),
+                    GPR_DUMP_HEX | GPR_DUMP_ASCII, "01 '.'");
 }
 
 static void test_pu32_fail(const char *s) {
@@ -148,7 +176,8 @@ static void test_asprintf(void) {
 int main(int argc, char **argv) {
   grpc_test_init(argc, argv);
   test_strdup();
-  test_hexdump();
+  test_dump();
+  test_dump_slice();
   test_parse_uint32();
   test_asprintf();
   return 0;
diff --git a/test/core/transport/chttp2/bin_encoder_test.c b/test/core/transport/chttp2/bin_encoder_test.c
index 983eaf5a0d..1ffd8ed3cb 100644
--- a/test/core/transport/chttp2/bin_encoder_test.c
+++ b/test/core/transport/chttp2/bin_encoder_test.c
@@ -44,10 +44,8 @@ static int all_ok = 1;
 static void expect_slice_eq(gpr_slice expected, gpr_slice slice, char *debug,
                             int line) {
   if (0 != gpr_slice_cmp(slice, expected)) {
-    char *hs = gpr_hexdump((const char *)GPR_SLICE_START_PTR(slice),
-                           GPR_SLICE_LENGTH(slice), GPR_HEXDUMP_PLAINTEXT);
-    char *he = gpr_hexdump((const char *)GPR_SLICE_START_PTR(expected),
-                           GPR_SLICE_LENGTH(expected), GPR_HEXDUMP_PLAINTEXT);
+    char *hs = gpr_dump_slice(slice, GPR_DUMP_HEX | GPR_DUMP_ASCII);
+    char *he = gpr_dump_slice(expected, GPR_DUMP_HEX | GPR_DUMP_ASCII);
     gpr_log(GPR_ERROR, "FAILED:%d: %s\ngot:  %s\nwant: %s", line, debug, hs,
             he);
     gpr_free(hs);
@@ -83,12 +81,9 @@ static void expect_combined_equiv(const char *s, size_t len, int line) {
   gpr_slice expect = grpc_chttp2_huffman_compress(base64);
   gpr_slice got = grpc_chttp2_base64_encode_and_huffman_compress(input);
   if (0 != gpr_slice_cmp(expect, got)) {
-    char *t = gpr_hexdump((const char *)GPR_SLICE_START_PTR(input),
-                          GPR_SLICE_LENGTH(input), GPR_HEXDUMP_PLAINTEXT);
-    char *e = gpr_hexdump((const char *)GPR_SLICE_START_PTR(expect),
-                          GPR_SLICE_LENGTH(expect), GPR_HEXDUMP_PLAINTEXT);
-    char *g = gpr_hexdump((const char *)GPR_SLICE_START_PTR(got),
-                          GPR_SLICE_LENGTH(got), GPR_HEXDUMP_PLAINTEXT);
+    char *t = gpr_dump_slice(input, GPR_DUMP_HEX | GPR_DUMP_ASCII);
+    char *e = gpr_dump_slice(expect, GPR_DUMP_HEX | GPR_DUMP_ASCII);
+    char *g = gpr_dump_slice(got, GPR_DUMP_HEX | GPR_DUMP_ASCII);
     gpr_log(GPR_ERROR, "FAILED:%d:\ntest: %s\ngot:  %s\nwant: %s", t, g, e);
     gpr_free(t);
     gpr_free(e);
diff --git a/test/core/transport/chttp2/stream_encoder_test.c b/test/core/transport/chttp2/stream_encoder_test.c
index bf70d43e78..6f2ea274fe 100644
--- a/test/core/transport/chttp2/stream_encoder_test.c
+++ b/test/core/transport/chttp2/stream_encoder_test.c
@@ -85,12 +85,8 @@ static void verify_sopb(size_t window_available, int eof,
   grpc_sopb_destroy(&encops);
 
   if (0 != gpr_slice_cmp(merged, expect)) {
-    char *expect_str =
-        gpr_hexdump((char *)GPR_SLICE_START_PTR(expect),
-                    GPR_SLICE_LENGTH(expect), GPR_HEXDUMP_PLAINTEXT);
-    char *got_str =
-        gpr_hexdump((char *)GPR_SLICE_START_PTR(merged),
-                    GPR_SLICE_LENGTH(merged), GPR_HEXDUMP_PLAINTEXT);
+    char *expect_str = gpr_dump_slice(expect, GPR_DUMP_HEX | GPR_DUMP_ASCII);
+    char *got_str = gpr_dump_slice(merged, GPR_DUMP_HEX | GPR_DUMP_ASCII);
     gpr_log(GPR_ERROR, "mismatched output for %s", expected);
     gpr_log(GPR_ERROR, "EXPECT: %s", expect_str);
     gpr_log(GPR_ERROR, "GOT:    %s", got_str);