]> granicus.if.org Git - graphviz/commitdiff
smyrna load_attributes: use a string view for 'ss'
authorMatthew Fernandez <matthew.fernandez@gmail.com>
Sun, 10 Jul 2022 18:03:02 +0000 (11:03 -0700)
committerMatthew Fernandez <matthew.fernandez@gmail.com>
Sat, 16 Jul 2022 00:06:35 +0000 (17:06 -0700)
This code contained multiple memory leaks and unchecked allocations:¹

  1. `pch` was `strdup`-ed into `ss` on line 58. But `strdup`-ed again when
     being saved to an `attr` field. This lost the memory originating from the
     first `strdup`.

  2. Cases 0, 3, and 4 of the switch do not save the full contents of `ss` at
     all. This means naively removing the `strdup` calls in cases 1, 2, and
     default would not have solved the memory leak in (1) because cases 0, 3,
     and 4 would still leak memory.

  3. None of the `strdup` calls in this function were checked for failure.

This commit attempts to solve all the above. We now take a read-only reference
to the string data on line 58 and only `strdup` it when needed.

¹ It also assumes all lines of the input file are fewer characters than
  `BUFSIZ`, a platform-dependent constant. I do not know why this would be
  guaranteed. However, this problem seems orthogonal to the above.

cmd/smyrna/gui/gui.c

index a082672c744b6a7721357c9992d13b3f17f6a624..c82863f9f89671aa7abde8a6b9c4a5d7104407ca 100644 (file)
@@ -15,6 +15,7 @@
 #include <gdk/gdkkeysyms.h>
 #include <gdk/gdk.h>
 #include "viewport.h"
+#include <cgraph/strview.h>
 #include <common/memory.h>
 
 GladeXML *xml;                 //global libglade vars
@@ -42,7 +43,6 @@ void load_attributes(void)
 {
     FILE *file;
     char line[BUFSIZ];
-    char *ss;
     char *pch;
     static char *smyrna_attrs;
 
@@ -55,22 +55,22 @@ void load_attributes(void)
        for (int attrcount = 0; fgets(line, sizeof line, file) != NULL; ++attrcount) {
            pch = strtok(line, ",");
            for (int ind = 0; pch != NULL; ++ind) {
-               ss = strdup(pch);
+               strview_t ss = strview(pch, '\0');
                pch = strtok(NULL, ",");
                switch (ind) {
                case 0:
-                   attr[attrcount].Type = ss[0];
+                   attr[attrcount].Type = ss.size > 0 ? ss.data[0] : '\0';
                    break;
                case 1:
-                   attr[attrcount].Name = strdup(ss);
+                   attr[attrcount].Name = strview_str(ss);
                    break;
                case 2:
-                   attr[attrcount].Default = strdup(ss);
+                   attr[attrcount].Default = strview_str(ss);
                    break;
                case 3:
                    break;
                case 4:
-                   if (strstr(ss, "ALL_ENGINES")) {
+                   if (strview_str_contains(ss, "ALL_ENGINES")) {
                        attr[attrcount].Engine[GVK_DOT] = 1;
                        attr[attrcount].Engine[GVK_NEATO] = 1;
                        attr[attrcount].Engine[GVK_TWOPI] = 1;
@@ -78,15 +78,15 @@ void load_attributes(void)
                        attr[attrcount].Engine[GVK_FDP] = 1;
                    } else {
                        attr[attrcount].Engine[GVK_DOT] =
-                           strstr(ss, "DOT") ? 1 : 0;
+                           strview_str_contains(ss, "DOT") ? 1 : 0;
                        attr[attrcount].Engine[GVK_NEATO] =
-                           strstr(ss, "NEATO") ? 1 : 0;
+                           strview_str_contains(ss, "NEATO") ? 1 : 0;
                        attr[attrcount].Engine[GVK_TWOPI] =
-                           strstr(ss, "TWOPI") ? 1 : 0;
+                           strview_str_contains(ss, "TWOPI") ? 1 : 0;
                        attr[attrcount].Engine[GVK_CIRCO] =
-                           strstr(ss, "CIRCO") ? 1 : 0;
+                           strview_str_contains(ss, "CIRCO") ? 1 : 0;
                        attr[attrcount].Engine[GVK_FDP] =
-                           strstr(ss, "FDP") ? 1 : 0;
+                           strview_str_contains(ss, "FDP") ? 1 : 0;
                    }
                    break;
                default:
@@ -94,8 +94,7 @@ void load_attributes(void)
                        RALLOC(attr[attrcount].ComboValuesCount,
                               attr[attrcount].ComboValues, char *);
                    attr[attrcount].ComboValues[attr[attrcount].
-                                               ComboValuesCount] =
-                       strdup(ss);
+                                               ComboValuesCount] = strview_str(ss);
                    attr[attrcount].ComboValuesCount++;
                    break;
                }