Hi, Thanks for your contribution. Although some of these might be genuine leaks, they mostly occur of you use the same option multiple times in the configuration file. On Wed, 2009-05-06 at 08:26 +1200, mark tomlinson wrote:

Fix various memory leaks caused by strings not being freed. Also, in log_source_group_deinit(), ensure no drivers are still pointing to this possibly soon-to-be-freed structure. Without this a SIGSEGV was possible when a SIGHUP occured while syslog was busy.

diff --git a/src/afinet.c b/src/afinet.c index 28b9d50..144a08a 100644 --- a/src/afinet.c +++ b/src/afinet.c @@ -191,6 +191,8 @@ afinet_sd_set_transport(LogDriver *s, const gchar *transport) { AFInetSourceDriver *self = (AFInetSourceDriver *) s;

+ if (self->super.transport) + g_free(self->super.transport); self->super.transport = g_strdup(transport); if (strcasecmp(transport, "udp") == 0) {

this leak only occurs, if you use multiple transport() options.

diff --git a/src/afstreams.c b/src/afstreams.c index df4e758..35d4812 100644 --- a/src/afstreams.c +++ b/src/afstreams.c @@ -104,6 +104,8 @@ afstreams_sd_set_sundoor(LogDriver *s, gchar *filename) { AFStreamsSourceDriver *self = (AFStreamsSourceDriver *) s;

+ if (self->door_filename) + g_string_free(self->door_filename, TRUE); self->door_filename = g_string_new(filename); }

would be better to use g_string_assign() in this case, but I'll take care about it.

diff --git a/src/dgroup.c b/src/dgroup.c index 7919a54..be02899 100644 --- a/src/dgroup.c +++ b/src/dgroup.c @@ -76,6 +76,16 @@ log_dest_group_deinit(LogPipe *s) NULL); success = FALSE; } + if (p->group) + { + g_free(p->group); + p->group = NULL; + } + if (p->id) + { + g_free(p->id); + p->id = NULL; + } } return success; }

these should be freed when the driver in question gets freed in the log_drv_free() function

diff --git a/src/logsource.c b/src/logsource.c index 311e61f..93a8747 100644 --- a/src/logsource.c +++ b/src/logsource.c @@ -181,6 +181,10 @@ log_source_set_options(LogSource *self, LogSourceOptions *options, gint stats_le g_atomic_counter_set(&self->options->window_size, current_window); self->stats_level = stats_level; self->stats_source = stats_source; + if (self->stats_id) + g_free(self->stats_id); + if (self->stats_instance) + g_free(self->stats_instance); self->stats_id = stats_id ? g_strdup(stats_id) : NULL; self->stats_instance = stats_instance ? g_strdup(stats_instance): NULL; }

diff --git a/src/logwriter.c b/src/logwriter.c index 4c895cc..1d1e00e 100644 --- a/src/logwriter.c +++ b/src/logwriter.c @@ -792,6 +792,10 @@ log_writer_set_options(LogWriter *self, LogPipe *control, LogWriterOptions *opti

self->stats_level = stats_level; self->stats_source = stats_source; + if (self->stats_id) + g_free(self->stats_id); + if (self->stats_instance) + g_free(self->stats_instance); self->stats_id = stats_id ? g_strdup(stats_id) : NULL; self->stats_instance = stats_instance ? g_strdup(stats_instance) : NULL;

these again should only be called once, but better if the function is idempotent.

diff --git a/src/sgroup.c b/src/sgroup.c index 904237c..0fdf8fb 100644 --- a/src/sgroup.c +++ b/src/sgroup.c @@ -79,7 +79,18 @@ log_source_group_deinit(LogPipe *s) evt_tag_str("id", p->id), NULL); success = FALSE; - } + } + if (p->group) + { + g_free(p->group); + p->group = NULL; + } + if (p->id) + { + g_free(p->id); + p->id = NULL; + } + log_pipe_append(&p->super, NULL);

hmm... can you show me the backtrace of the crash that this is supposed to fix? when the configuration is deinitialized the old LogSource instances should never be referenced again, thus this dangling pointer will never be referenced again. Also, group/id is freed whenever the driver instance is freed, thus no need to open code it. I see the possibility of a leak if syslog-ng falls back to the old configuration as we coulnd't initialize the new one (in which case log_sgroup_init is called multiple times on the same config), but otherwise it should not be a leak. -- Bazsi