add utility function for turning integers into strings

This is common operation within Graphviz. It is also a common source of bugs.
There have been numerous instances of ints being stringized into fixed sized
buffers that were not large enough to hold them. The result has been pernicious
memory safety issues that have been difficult to reproduce and root cause.¹

The hope is that this wrapper can provide a safe, efficient, and tested
alternative. Going forwards, the intent is to replace as many compatible
operations as possible with calls to this function.

  ¹ A user would report a crash that relied on their objects having large IDs or
    their platform having a large int width. The example would run fine in a
    different environment and mystify the maintainers.

diff --git a/lib/cgraph/CMakeLists.txt b/lib/cgraph/CMakeLists.txt
index 9f6a6a643f9f344f4efda77f85c994ffff60bf6a..f52e93d9a68381c4478525a47631c58ffadd404e 100644 (file)
--- a/lib/cgraph/CMakeLists.txt
+++ b/lib/cgraph/CMakeLists.txt
@@ -9,6 +9,7 @@ add_library(cgraph SHARED
     agxbuf.h
     cghdr.h
     cgraph.h
+    itos.h
     sprint.h
     strcasecmp.h
 

diff --git a/lib/cgraph/Makefile.am b/lib/cgraph/Makefile.am
index 79c1c862510bbd927596400e4f771b324ba84fa7..9d520452577937fcaa6f50ffbc003831a42c02fe 100644 (file)
--- a/lib/cgraph/Makefile.am
+++ b/lib/cgraph/Makefile.am
@@ -8,7 +8,7 @@ pkgconfigdir = $(libdir)/pkgconfig
 AM_CPPFLAGS = -I$(top_srcdir)/lib -I$(top_srcdir)/lib/cdt
 
 pkginclude_HEADERS = cgraph.h
-noinst_HEADERS = agxbuf.h cghdr.h sprint.h strcasecmp.h
+noinst_HEADERS = agxbuf.h cghdr.h itos.h sprint.h strcasecmp.h
 noinst_LTLIBRARIES = libcgraph_C.la
 lib_LTLIBRARIES = libcgraph.la
 pkgconfig_DATA = libcgraph.pc

diff --git a/lib/cgraph/cgraph.vcxproj b/lib/cgraph/cgraph.vcxproj
index 4d171146042b45770521adb153ad58d4fa61c794..7fa787f35df31a1624b9eec5a7a080b3f680c8d7 100644 (file)
--- a/lib/cgraph/cgraph.vcxproj
+++ b/lib/cgraph/cgraph.vcxproj
@@ -101,6 +101,7 @@ win_flex -oscan.c scan.l</Command>
     <ClInclude Include="agxbuf.h" />
     <ClInclude Include="cghdr.h" />
     <ClInclude Include="cgraph.h" />
+    <ClInclude Include="itos.h" />
     <ClInclude Include="sprint.h" />
     <ClInclude Include="strcasecmp.h" />
   </ItemGroup>

diff --git a/lib/cgraph/cgraph.vcxproj.filters b/lib/cgraph/cgraph.vcxproj.filters
index 18099af3327e7f8bef616fddb0d2ab7e51f43a19..f6d120770c043bf7606a17278dbc6a41f4ac989d 100644 (file)
--- a/lib/cgraph/cgraph.vcxproj.filters
+++ b/lib/cgraph/cgraph.vcxproj.filters
@@ -24,6 +24,9 @@
     <ClInclude Include="cgraph.h">
       <Filter>Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="itos.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
     <ClInclude Include="sprint.h">
       <Filter>Header Files</Filter>
     </ClInclude>

diff --git a/lib/cgraph/itos.h b/lib/cgraph/itos.h
new file mode 100644 (file)
index 0000000..6f3cefd
--- /dev/null
+++ b/lib/cgraph/itos.h
@@ -0,0 +1,22 @@
+#pragma once
+
+#include <stdio.h>
+
+// return type of itos below
+struct itos_ {
+  char str[12];
+};
+
+/** convert an integer to a string
+ *
+ * Intended usage is something like:
+ *   agattr(g, AGEDGE, "hello", itos(42).str)
+ *
+ * @param i Integer to convert
+ * @return Stringized conversion wrapped in a struct
+ */
+static inline struct itos_ itos(int i) {
+  struct itos_ s;
+  (void)snprintf(s.str, sizeof(s.str), "%d", i);
+  return s;
+}

diff --git a/lib/cgraph/test_itos.c b/lib/cgraph/test_itos.c
new file mode 100644 (file)
index 0000000..842bf59
--- /dev/null
+++ b/lib/cgraph/test_itos.c
@@ -0,0 +1,75 @@
+// basic unit tester for itos
+
+#ifdef NDEBUG
+#error this is not intended to be compiled with assertions off
+#endif
+
+#include <assert.h>
+#include <cgraph/itos.h>
+#include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+static void test_0(void) {
+  assert(strcmp(itos(0).str, "0") == 0);
+}
+
+static void test_1(void) {
+  assert(strcmp(itos(1).str, "1") == 0);
+}
+
+static void test_neg_1(void) {
+  assert(strcmp(itos(-1).str, "-1") == 0);
+}
+
+static void test_min(void) {
+
+  int r = snprintf(NULL, 0, "%d", INT_MIN);
+  assert(r > 0);
+
+  char *buffer = malloc(sizeof(char) * ((size_t)r + 1));
+  assert(buffer != NULL);
+
+  (void)sprintf(buffer, "%d", INT_MIN);
+
+  assert(strcmp(itos(INT_MIN).str, buffer) == 0);
+
+  free(buffer);
+}
+
+static void test_max(void) {
+
+  int r = snprintf(NULL, 0, "%d", INT_MAX);
+  assert(r > 0);
+
+  char *buffer = malloc(sizeof(char) * ((size_t)r + 1));
+  assert(buffer != NULL);
+
+  (void)sprintf(buffer, "%d", INT_MAX);
+
+  assert(strcmp(itos(INT_MAX).str, buffer) == 0);
+
+  free(buffer);
+}
+
+int main(void) {
+
+#define RUN(t)                                                                 \
+  do {                                                                         \
+    printf("running test_%s... ", #t);                                         \
+    fflush(stdout);                                                            \
+    test_##t();                                                                \
+    printf("OK\n");                                                            \
+  } while (0)
+
+  RUN(0);
+  RUN(1);
+  RUN(neg_1);
+  RUN(min);
+  RUN(max);
+
+#undef RUN
+
+  return EXIT_SUCCESS;
+}

diff --git a/rtest/test_itos.py b/rtest/test_itos.py
new file mode 100644 (file)
index 0000000..55ca603
--- /dev/null
+++ b/rtest/test_itos.py
@@ -0,0 +1,25 @@
+"""test ../lib/cgraph/itos.h"""
+
+import os
+from pathlib import Path
+import sys
+
+sys.path.append(os.path.dirname(__file__))
+from gvtest import run_c
+
+def test_itos():
+  """run the itos unit tests"""
+
+  # locate the itos unit tests
+  src = Path(__file__).parent.resolve() / "../lib/cgraph/test_itos.c"
+  assert src.exists()
+
+  # locate lib directory that needs to be in the include path
+  lib = Path(__file__).parent.resolve() / "../lib"
+
+  # extra C flags this compilation needs
+  cflags = ['-I', lib]
+
+  ret, _, _ = run_c(src, cflags=cflags)
+
+  assert ret == 0