print_to_socket() simply returns from current function always when it was not able to write to the socket. If this happens variables will not be freed. This patch changes all unixsocket command functions in order to use a simple garbage collector, which free's all variables whenever the function returns. Whenever memory will be allocated the allocated variable will be registered within garbage collector. --- src/utils_cmd_flush.c | 36 +++++++++++++++----------- src/utils_cmd_getthreshold.c | 58 +++++++++++++++++++++++++------------------ src/utils_cmd_getval.c | 50 +++++++++++++++++++++---------------- src/utils_cmd_listval.c | 33 ++++++++++++++++-------- src/utils_cmd_putval.c | 46 ++++++++++++++++++++-------------- 5 files changed, 135 insertions(+), 88 deletions(-) Index: collectd-4.9.1/src/utils_cmd_flush.c =================================================================== --- collectd-4.9.1.orig/src/utils_cmd_flush.c 2010-02-05 21:39:35.000000000 +0100 +++ collectd-4.9.1/src/utils_cmd_flush.c 2010-02-05 22:00:25.000000000 +0100 @@ -26,11 +26,12 @@ #include "plugin.h" #include "utils_parse_option.h" -#define print_to_socket(fh, ...) \ +#define print_to_socket(fh, gc, ...) \ if (fprintf (fh, __VA_ARGS__) < 0) { \ char errbuf[1024]; \ WARNING ("handle_flush: failed to write to socket #%i: %s", \ fileno (fh), sstrerror (errno, errbuf, sizeof (errbuf))); \ + gc_free(gc); \ return -1; \ } @@ -62,15 +63,19 @@ int i; + struct gc_list *gc; + if ((fh == NULL) || (buffer == NULL)) return (-1); + gc = gc_init(); DEBUG ("utils_cmd_flush: handle_flush (fh = %p, buffer = %s);", (void *) fh, buffer); if (strncasecmp ("FLUSH", buffer, strlen ("FLUSH")) != 0) { - print_to_socket (fh, "-1 Cannot parse command.\n"); + print_to_socket (fh, gc, "-1 Cannot parse command.\n"); + gc_free(gc); return (-1); } buffer += strlen ("FLUSH"); @@ -81,14 +86,18 @@ char *opt_value; int status; + if (plugins != NULL) + gc_register(gc, plugins); + if (identifiers != NULL) + gc_register(gc, identifiers); + opt_key = NULL; opt_value = NULL; status = parse_option (&buffer, &opt_key, &opt_value); if (status != 0) { - print_to_socket (fh, "-1 Parsing options failed.\n"); - sfree (plugins); - sfree (identifiers); + print_to_socket (fh, gc, "-1 Parsing options failed.\n"); + gc_free(gc); return (-1); } @@ -110,10 +119,9 @@ if ((endptr == opt_value) || (errno != 0)) { - print_to_socket (fh, "-1 Invalid value for option `timeout': " + print_to_socket (fh, gc, "-1 Invalid value for option `timeout': " "%s\n", opt_value); - sfree (plugins); - sfree (identifiers); + gc_free(gc); return (-1); } else if (timeout <= 0) @@ -121,9 +129,8 @@ } else { - print_to_socket (fh, "-1 Cannot parse option %s\n", opt_key); - sfree (plugins); - sfree (identifiers); + print_to_socket (fh, gc, "-1 Cannot parse option %s\n", opt_key); + gc_free(gc); return (-1); } } /* while (*buffer != 0) */ @@ -159,17 +166,16 @@ if ((success + error) > 0) { - print_to_socket (fh, "0 Done: %i successful, %i errors\n", + print_to_socket (fh, gc, "0 Done: %i successful, %i errors\n", success, error); } else { plugin_flush (NULL, timeout, NULL); - print_to_socket (fh, "0 Done\n"); + print_to_socket (fh, gc, "0 Done\n"); } - sfree (plugins); - sfree (identifiers); + gc_free(gc); return (0); } /* int handle_flush */ Index: collectd-4.9.1/src/utils_cmd_listval.c =================================================================== --- collectd-4.9.1.orig/src/utils_cmd_listval.c 2010-02-05 21:39:35.000000000 +0100 +++ collectd-4.9.1/src/utils_cmd_listval.c 2010-02-05 21:59:09.000000000 +0100 @@ -27,11 +27,12 @@ #include "utils_cache.h" #include "utils_parse_option.h" -#define print_to_socket(fh, ...) \ +#define print_to_socket(fh, gc, ...) \ if (fprintf (fh, __VA_ARGS__) < 0) { \ char errbuf[1024]; \ WARNING ("handle_listval: failed to write to socket #%i: %s", \ fileno (fh), sstrerror (errno, errbuf, sizeof (errbuf))); \ + gc_free(gc); \ return -1; \ } @@ -44,6 +45,8 @@ size_t i; int status; + struct gc_list *gc = gc_init(); + DEBUG ("utils_cmd_listval: handle_listval (fh = %p, buffer = %s);", (void *) fh, buffer); @@ -51,42 +54,50 @@ status = parse_string (&buffer, &command); if (status != 0) { - print_to_socket (fh, "-1 Cannot parse command.\n"); + print_to_socket (fh, gc, "-1 Cannot parse command.\n"); + gc_free(gc); return (-1); } assert (command != NULL); if (strcasecmp ("LISTVAL", command) != 0) { - print_to_socket (fh, "-1 Unexpected command: `%s'.\n", command); + print_to_socket (fh, gc, "-1 Unexpected command: `%s'.\n", command); + gc_free(gc); return (-1); } if (*buffer != 0) { - print_to_socket (fh, "-1 Garbage after end of command: %s\n", buffer); + print_to_socket (fh, gc, "-1 Garbage after end of command: %s\n", buffer); + gc_free(gc); return (-1); } status = uc_get_names (&names, ×, &number); + gc_register(gc, names); + gc_register(gc, times); + for (i = 0; i < number; i++) + { + gc_register(gc, names[i]); + } + if (status != 0) { DEBUG ("command listval: uc_get_names failed with status %i", status); - print_to_socket (fh, "-1 uc_get_names failed.\n"); + print_to_socket (fh, gc, "-1 uc_get_names failed.\n"); + gc_free(gc); return (-1); } - print_to_socket (fh, "%i Value%s found\n", + print_to_socket (fh, gc, "%i Value%s found\n", (int) number, (number == 1) ? "" : "s"); for (i = 0; i < number; i++) { - print_to_socket (fh, "%u %s\n", (unsigned int) times[i], names[i]); - sfree(names[i]); + print_to_socket (fh, gc, "%u %s\n", (unsigned int) times[i], names[i]); } - sfree(names); - sfree(times); - + gc_free(gc); return (0); } /* int handle_listval */ Index: collectd-4.9.1/src/utils_cmd_getthreshold.c =================================================================== --- collectd-4.9.1.orig/src/utils_cmd_getthreshold.c 2010-02-05 21:39:35.000000000 +0100 +++ collectd-4.9.1/src/utils_cmd_getthreshold.c 2010-02-05 22:21:04.000000000 +0100 @@ -27,11 +27,12 @@ #include "utils_parse_option.h" /* for `parse_string' */ #include "utils_cmd_getthreshold.h" -#define print_to_socket(fh, ...) \ +#define print_to_socket(fh, gc, ...) \ if (fprintf (fh, __VA_ARGS__) < 0) { \ char errbuf[1024]; \ WARNING ("handle_getthreshold: failed to write to socket #%i: %s", \ fileno (fh), sstrerror (errno, errbuf, sizeof (errbuf))); \ + gc_free(gc); \ return -1; \ } @@ -53,9 +54,13 @@ int status; size_t i; + struct gc_list *gc; + if ((fh == NULL) || (buffer == NULL)) return (-1); + gc = gc_init(); + DEBUG ("utils_cmd_getthreshold: handle_getthreshold (fh = %p, buffer = %s);", (void *) fh, buffer); @@ -63,14 +68,16 @@ status = parse_string (&buffer, &command); if (status != 0) { - print_to_socket (fh, "-1 Cannot parse command.\n"); + print_to_socket (fh, gc, "-1 Cannot parse command.\n"); + gc_free(gc); return (-1); } assert (command != NULL); if (strcasecmp ("GETTHRESHOLD", command) != 0) { - print_to_socket (fh, "-1 Unexpected command: `%s'.\n", command); + print_to_socket (fh, gc, "-1 Unexpected command: `%s'.\n", command); + gc_free(gc); return (-1); } @@ -78,20 +85,23 @@ status = parse_string (&buffer, &identifier); if (status != 0) { - print_to_socket (fh, "-1 Cannot parse identifier.\n"); + print_to_socket (fh, gc, "-1 Cannot parse identifier.\n"); + gc_free(gc); return (-1); } assert (identifier != NULL); if (*buffer != 0) { - print_to_socket (fh, "-1 Garbage after end of command: %s\n", buffer); + print_to_socket (fh, gc, "-1 Garbage after end of command: %s\n", buffer); + gc_free(gc); return (-1); } /* parse_identifier() modifies its first argument, * returning pointers into it */ identifier_copy = sstrdup (identifier); + gc_register(gc, identifier_copy); status = parse_identifier (identifier_copy, &host, &plugin, &plugin_instance, @@ -99,8 +109,8 @@ if (status != 0) { DEBUG ("handle_getthreshold: Cannot parse identifier `%s'.", identifier); - print_to_socket (fh, "-1 Cannot parse identifier `%s'.\n", identifier); - sfree (identifier_copy); + print_to_socket (fh, gc, "-1 Cannot parse identifier `%s'.\n", identifier); + gc_free(gc); return (-1); } @@ -112,20 +122,21 @@ sstrncpy (vl.type, type, sizeof (vl.type)); if (type_instance != NULL) sstrncpy (vl.type_instance, type_instance, sizeof (vl.type_instance)); - sfree (identifier_copy); memset (&threshold, 0, sizeof (threshold)); status = ut_search_threshold (&vl, &threshold); if (status == ENOENT) { - print_to_socket (fh, "-1 No threshold found for identifier %s\n", + print_to_socket (fh, gc, "-1 No threshold found for identifier %s\n", identifier); + gc_free(gc); return (0); } else if (status != 0) { - print_to_socket (fh, "-1 Error while looking up threshold: %i\n", + print_to_socket (fh, gc, "-1 Error while looking up threshold: %i\n", status); + gc_free(gc); return (-1); } @@ -145,33 +156,34 @@ if (threshold.hits > 1) i++; /* Print the response */ - print_to_socket (fh, "%zu Threshold found\n", i); + print_to_socket (fh, gc, "%zu Threshold found\n", i); if (threshold.host[0] != 0) - print_to_socket (fh, "Host: %s\n", threshold.host) + print_to_socket (fh, gc, "Host: %s\n", threshold.host) if (threshold.plugin[0] != 0) - print_to_socket (fh, "Plugin: %s\n", threshold.plugin) + print_to_socket (fh, gc, "Plugin: %s\n", threshold.plugin) if (threshold.plugin_instance[0] != 0) - print_to_socket (fh, "Plugin Instance: %s\n", threshold.plugin_instance) + print_to_socket (fh, gc, "Plugin Instance: %s\n", threshold.plugin_instance) if (threshold.type[0] != 0) - print_to_socket (fh, "Type: %s\n", threshold.type) + print_to_socket (fh, gc, "Type: %s\n", threshold.type) if (threshold.type_instance[0] != 0) - print_to_socket (fh, "Type Instance: %s\n", threshold.type_instance) + print_to_socket (fh, gc, "Type Instance: %s\n", threshold.type_instance) if (threshold.data_source[0] != 0) - print_to_socket (fh, "Data Source: %s\n", threshold.data_source) + print_to_socket (fh, gc, "Data Source: %s\n", threshold.data_source) if (!isnan (threshold.warning_min)) - print_to_socket (fh, "Warning Min: %g\n", threshold.warning_min) + print_to_socket (fh, gc, "Warning Min: %g\n", threshold.warning_min) if (!isnan (threshold.warning_max)) - print_to_socket (fh, "Warning Max: %g\n", threshold.warning_max) + print_to_socket (fh, gc, "Warning Max: %g\n", threshold.warning_max) if (!isnan (threshold.failure_min)) - print_to_socket (fh, "Failure Min: %g\n", threshold.failure_min) + print_to_socket (fh, gc, "Failure Min: %g\n", threshold.failure_min) if (!isnan (threshold.failure_max)) - print_to_socket (fh, "Failure Max: %g\n", threshold.failure_max) + print_to_socket (fh, gc, "Failure Max: %g\n", threshold.failure_max) if (threshold.hysteresis > 0.0) - print_to_socket (fh, "Hysteresis: %g\n", threshold.hysteresis) + print_to_socket (fh, gc, "Hysteresis: %g\n", threshold.hysteresis) if (threshold.hits > 1) - print_to_socket (fh, "Hits: %i\n", threshold.hits) + print_to_socket (fh, gc, "Hits: %i\n", threshold.hits) + gc_free(gc); return (0); } /* int handle_getthreshold */ Index: collectd-4.9.1/src/utils_cmd_getval.c =================================================================== --- collectd-4.9.1.orig/src/utils_cmd_getval.c 2010-02-05 21:39:35.000000000 +0100 +++ collectd-4.9.1/src/utils_cmd_getval.c 2010-02-05 22:25:36.000000000 +0100 @@ -26,11 +26,12 @@ #include "utils_cache.h" #include "utils_parse_option.h" -#define print_to_socket(fh, ...) \ +#define print_to_socket(fh, gc, ...) \ if (fprintf (fh, __VA_ARGS__) < 0) { \ char errbuf[1024]; \ WARNING ("handle_getval: failed to write to socket #%i: %s", \ fileno (fh), sstrerror (errno, errbuf, sizeof (errbuf))); \ + gc_free(gc); \ return -1; \ } @@ -53,9 +54,13 @@ int status; size_t i; + struct gc_list *gc; + if ((fh == NULL) || (buffer == NULL)) return (-1); + gc = gc_init(); + DEBUG ("utils_cmd_getval: handle_getval (fh = %p, buffer = %s);", (void *) fh, buffer); @@ -63,14 +68,16 @@ status = parse_string (&buffer, &command); if (status != 0) { - print_to_socket (fh, "-1 Cannot parse command.\n"); + print_to_socket (fh, gc, "-1 Cannot parse command.\n"); + gc_free(gc); return (-1); } assert (command != NULL); if (strcasecmp ("GETVAL", command) != 0) { - print_to_socket (fh, "-1 Unexpected command: `%s'.\n", command); + print_to_socket (fh, gc, "-1 Unexpected command: `%s'.\n", command); + gc_free(gc); return (-1); } @@ -78,20 +85,23 @@ status = parse_string (&buffer, &identifier); if (status != 0) { - print_to_socket (fh, "-1 Cannot parse identifier.\n"); + print_to_socket (fh, gc, "-1 Cannot parse identifier.\n"); + gc_free(gc); return (-1); } assert (identifier != NULL); if (*buffer != 0) { - print_to_socket (fh, "-1 Garbage after end of command: %s\n", buffer); + print_to_socket (fh, gc, "-1 Garbage after end of command: %s\n", buffer); + gc_free(gc); return (-1); } /* parse_identifier() modifies its first argument, * returning pointers into it */ identifier_copy = sstrdup (identifier); + gc_register(gc, identifier_copy); status = parse_identifier (identifier_copy, &hostname, &plugin, &plugin_instance, @@ -99,8 +109,8 @@ if (status != 0) { DEBUG ("handle_getval: Cannot parse identifier `%s'.", identifier); - print_to_socket (fh, "-1 Cannot parse identifier `%s'.\n", identifier); - sfree (identifier_copy); + print_to_socket (fh, gc, "-1 Cannot parse identifier `%s'.\n", identifier); + gc_free(gc); return (-1); } @@ -108,18 +118,19 @@ if (ds == NULL) { DEBUG ("handle_getval: plugin_get_ds (%s) == NULL;", type); - print_to_socket (fh, "-1 Type `%s' is unknown.\n", type); - sfree (identifier_copy); + print_to_socket (fh, gc, "-1 Type `%s' is unknown.\n", type); + gc_free(gc); return (-1); } values = NULL; values_num = 0; status = uc_get_rate_by_name (identifier, &values, &values_num); + gc_register(gc, values); if (status != 0) { - print_to_socket (fh, "-1 No such value\n"); - sfree (identifier_copy); + print_to_socket (fh, gc, "-1 No such value\n"); + gc_free(gc); return (-1); } @@ -128,30 +139,27 @@ ERROR ("ds[%s]->ds_num = %i, " "but uc_get_rate_by_name returned %u values.", ds->type, ds->ds_num, (unsigned int) values_num); - print_to_socket (fh, "-1 Error reading value from cache.\n"); - sfree (values); - sfree (identifier_copy); + print_to_socket (fh, gc, "-1 Error reading value from cache.\n"); + gc_free(gc); return (-1); } - print_to_socket (fh, "%u Value%s found\n", (unsigned int) values_num, + print_to_socket (fh, gc, "%u Value%s found\n", (unsigned int) values_num, (values_num == 1) ? "" : "s"); for (i = 0; i < values_num; i++) { - print_to_socket (fh, "%s=", ds->ds[i].name); + print_to_socket (fh, gc, "%s=", ds->ds[i].name); if (isnan (values[i])) { - print_to_socket (fh, "NaN\n"); + print_to_socket (fh, gc, "NaN\n"); } else { - print_to_socket (fh, "%12e\n", values[i]); + print_to_socket (fh, gc, "%12e\n", values[i]); } } - sfree (values); - sfree (identifier_copy); - + gc_free(gc); return (0); } /* int handle_getval */ Index: collectd-4.9.1/src/utils_cmd_putval.c =================================================================== --- collectd-4.9.1.orig/src/utils_cmd_putval.c 2010-02-05 21:39:35.000000000 +0100 +++ collectd-4.9.1/src/utils_cmd_putval.c 2010-02-05 22:18:41.000000000 +0100 @@ -25,11 +25,12 @@ #include "utils_parse_option.h" -#define print_to_socket(fh, ...) \ +#define print_to_socket(fh, gc, ...) \ if (fprintf (fh, __VA_ARGS__) < 0) { \ char errbuf[1024]; \ WARNING ("handle_putval: failed to write to socket #%i: %s", \ fileno (fh), sstrerror (errno, errbuf, sizeof (errbuf))); \ + gc_free(gc); \ return -1; \ } @@ -41,7 +42,7 @@ status = parse_values (buffer, vl, ds); if (status != 0) { - print_to_socket (fh, "-1 Parsing the values string failed.\n"); + print_to_socket (fh, NULL, "-1 Parsing the values string failed.\n"); return (-1); } @@ -90,6 +91,8 @@ const data_set_t *ds; value_list_t vl = VALUE_LIST_INIT; + struct gc_list *gc = gc_init(); + DEBUG ("utils_cmd_putval: handle_putval (fh = %p, buffer = %s);", (void *) fh, buffer); @@ -97,14 +100,16 @@ status = parse_string (&buffer, &command); if (status != 0) { - print_to_socket (fh, "-1 Cannot parse command.\n"); + print_to_socket (fh, gc, "-1 Cannot parse command.\n"); + gc_free(gc); return (-1); } assert (command != NULL); if (strcasecmp ("PUTVAL", command) != 0) { - print_to_socket (fh, "-1 Unexpected command: `%s'.\n", command); + print_to_socket (fh, gc, "-1 Unexpected command: `%s'.\n", command); + gc_free(gc); return (-1); } @@ -112,7 +117,8 @@ status = parse_string (&buffer, &identifier); if (status != 0) { - print_to_socket (fh, "-1 Cannot parse identifier.\n"); + print_to_socket (fh, gc, "-1 Cannot parse identifier.\n"); + gc_free(gc); return (-1); } assert (identifier != NULL); @@ -120,6 +126,7 @@ /* parse_identifier() modifies its first argument, * returning pointers into it */ identifier_copy = sstrdup (identifier); + gc_register(gc, identifier_copy); status = parse_identifier (identifier_copy, &hostname, &plugin, &plugin_instance, @@ -128,9 +135,9 @@ { DEBUG ("handle_putval: Cannot parse identifier `%s'.", identifier); - print_to_socket (fh, "-1 Cannot parse identifier `%s'.\n", + print_to_socket (fh, gc, "-1 Cannot parse identifier `%s'.\n", identifier); - sfree (identifier_copy); + gc_free(gc); return (-1); } @@ -141,8 +148,8 @@ || ((type_instance != NULL) && (strlen (type_instance) >= sizeof (vl.type_instance)))) { - print_to_socket (fh, "-1 Identifier too long.\n"); - sfree (identifier_copy); + print_to_socket (fh, gc, "-1 Identifier too long.\n"); + gc_free(gc); return (-1); } @@ -156,8 +163,8 @@ ds = plugin_get_ds (type); if (ds == NULL) { - print_to_socket (fh, "-1 Type `%s' isn't defined.\n", type); - sfree (identifier_copy); + print_to_socket (fh, gc, "-1 Type `%s' isn't defined.\n", type); + gc_free(gc); return (-1); } @@ -165,13 +172,14 @@ hostname = NULL; plugin = NULL; plugin_instance = NULL; type = NULL; type_instance = NULL; - sfree (identifier_copy); vl.values_len = ds->ds_num; vl.values = (value_t *) malloc (vl.values_len * sizeof (value_t)); + gc_register(gc, vl.values); if (vl.values == NULL) { - print_to_socket (fh, "-1 malloc failed.\n"); + print_to_socket (fh, gc, "-1 malloc failed.\n"); + gc_free(gc); return (-1); } @@ -187,7 +195,8 @@ { /* parse_option failed, buffer has been modified. * => we need to abort */ - print_to_socket (fh, "-1 Misformatted option.\n"); + print_to_socket (fh, gc, "-1 Misformatted option.\n"); + gc_free(gc); return (-1); } else if (status == 0) @@ -203,7 +212,8 @@ status = parse_string (&buffer, &string); if (status != 0) { - print_to_socket (fh, "-1 Misformatted value.\n"); + print_to_socket (fh, gc, "-1 Misformatted value.\n"); + gc_free(gc); return (-1); } assert (string != NULL); @@ -212,18 +222,18 @@ if (status != 0) { /* An error has already been printed. */ + gc_free(gc); return (-1); } values_submitted++; } /* while (*buffer != 0) */ /* Done parsing the options. */ - print_to_socket (fh, "0 Success: %i %s been dispatched.\n", + print_to_socket (fh, gc, "0 Success: %i %s been dispatched.\n", values_submitted, (values_submitted == 1) ? "value has" : "values have"); - sfree (vl.values); - + gc_free(gc); return (0); } /* int handle_putval */ --