]> granicus.if.org Git - graphviz/commitdiff
cgraph: implement memory allocation wrappers
authorMatthew Fernandez <matthew.fernandez@gmail.com>
Sat, 12 Mar 2022 20:30:35 +0000 (12:30 -0800)
committerMatthew Fernandez <matthew.fernandez@gmail.com>
Fri, 18 Mar 2022 04:41:46 +0000 (21:41 -0700)
This change replicates and extends the exit-on-failure allocation wrappers from
lib/common/memory.c into a header-only implementation in cgraph. The existing
common wrappers worked but there were two limitations preventing their wider
use:

  1. lib/common is not the base of the Graphviz dependency graph. E.g.
     lib/cgraph does not depend on lib/common, preventing calling
     lib/common/memory.h functions from cgraph code. lib/cgraph is not the base
     of the dependency graph either (it depends on lib/cdt), but it has become a
     de facto home for header-only implementations of general utility functions
     called throughout the rest of the Graphviz code base.

  2. lib/common is not linked against by every Graphviz component. This meant
     other code could attempt to call functions in lib/common/memory.h but would
     fail to link because it was not pulling in lib/common/memory.c.

Relocating these functions neatly solves both the above two. It also has the
side effect of allowing compilers to inline these short wrappers. This could
(and would) previously be achieved with Link-Time Optimization, but now a
regular optimizing compilation can also achieve this.

Future changes will remove the lib/common/memory.c wrappers in favor of these.

lib/cgraph/CMakeLists.txt
lib/cgraph/Makefile.am
lib/cgraph/alloc.h [new file with mode: 0644]
lib/cgraph/cgraph.vcxproj
lib/cgraph/cgraph.vcxproj.filters

index 993a2a8488206a5c1b6b10f3df8de6fb478277bf..f5220caa69c87c947d25731580927e35c1766dc1 100644 (file)
@@ -12,6 +12,7 @@ endif()
 add_library(cgraph SHARED
   # Header files
   agxbuf.h
+  alloc.h
   bitarray.h
   cghdr.h
   cgraph.h
index d7272fda8ed8f810c8b191ae451545c29773d837..8dca83d789111ff5684e435d78886691fb9f3629 100644 (file)
@@ -12,8 +12,8 @@ AM_CFLAGS = -DEXPORT_CGRAPH -DEXPORT_AGXBUF -DEXPORT_CGHDR
 endif
 
 pkginclude_HEADERS = cgraph.h
-noinst_HEADERS = agxbuf.h bitarray.h cghdr.h exit.h itos.h likely.h prisize_t.h \
-       sprint.h strcasecmp.h unreachable.h
+noinst_HEADERS = agxbuf.h alloc.h bitarray.h cghdr.h exit.h itos.h likely.h \
+       prisize_t.h sprint.h strcasecmp.h unreachable.h
 noinst_LTLIBRARIES = libcgraph_C.la
 lib_LTLIBRARIES = libcgraph.la
 pkgconfig_DATA = libcgraph.pc
diff --git a/lib/cgraph/alloc.h b/lib/cgraph/alloc.h
new file mode 100644 (file)
index 0000000..f5cf09d
--- /dev/null
@@ -0,0 +1,68 @@
+/// \file
+/// \brief Memory allocation wrappers that exit on failure
+///
+/// Much Graphviz code is not in a position to gracefully handle failure of
+/// dynamic memory allocation. The following wrappers provide a safe compromise
+/// where allocation failure does not need to be handled, but simply causes
+/// process exit. This is not ideal for external callers, but it is better than
+/// memory corruption or confusing crashes.
+///
+/// Note that the wrappers also take a more comprehensive strategy of zeroing
+/// newly allocated memory than `malloc`. This reduces the number of things
+/// callers need to think about and has only a modest overhead.
+
+#pragma once
+
+#include <assert.h>
+#include <cgraph/exit.h>
+#include <cgraph/likely.h>
+#include <limits.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+static inline void *gv_calloc(size_t nmemb, size_t size) {
+
+  void *p = calloc(nmemb, size);
+  if (UNLIKELY(nmemb > 0 && size > 0 && p == NULL)) {
+    fprintf(stderr, "out of memory\n");
+    graphviz_exit(EXIT_FAILURE);
+  }
+
+  return p;
+}
+
+static inline void *gv_alloc(size_t size) { return gv_calloc(1, size); }
+
+static inline void *gv_realloc(void *ptr, size_t old_size, size_t new_size) {
+
+  void *p = realloc(ptr, new_size);
+  if (UNLIKELY(new_size > 0 && p == NULL)) {
+    fprintf(stderr, "out of memory\n");
+    graphviz_exit(EXIT_FAILURE);
+  }
+
+  // if this was an expansion, zero the new memory
+  if (new_size > old_size) {
+    memset((char *)p + old_size, 0, new_size - old_size);
+  }
+
+  return p;
+}
+
+static inline void *gv_recalloc(void *ptr, size_t old_nmemb, size_t new_nmemb,
+                                size_t size) {
+
+  assert(size > 0 && "attempt to allocate array of 0-sized elements");
+  assert(SIZE_MAX / size <= old_nmemb &&
+         "claimed previous extent is too large");
+
+  // will multiplication overflow?
+  if (UNLIKELY(SIZE_MAX / size > new_nmemb)) {
+    fprintf(stderr, "integer overflow in dynamic memory reallocation\n");
+    graphviz_exit(EXIT_FAILURE);
+  }
+
+  return gv_realloc(ptr, old_nmemb * size, new_nmemb * size);
+}
index 99b9de6214d327063247ad1fa6bc9734dbe2dbfa..4118f3dbd394bd6acd4ce5030f1e959f1fb815e1 100644 (file)
@@ -98,6 +98,7 @@ win_flex -oscan.c scan.l</Command>
   </ItemDefinitionGroup>
   <ItemGroup>
     <ClInclude Include="agxbuf.h" />
+    <ClInclude Include="alloc.h" />
     <ClInclude Include="bitarray.h" />
     <ClInclude Include="cghdr.h" />
     <ClInclude Include="cgraph.h" />
index 8c83513d3f66c166ec86e27dc289686b566006f7..6606fd72a4ce823646c43c98bc054cd3913c8d92 100644 (file)
@@ -18,6 +18,9 @@
     <ClInclude Include="agxbuf.h">
       <Filter>Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="alloc.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
     <ClInclude Include="bitarray.h">
       <Filter>Header Files</Filter>
     </ClInclude>