nagios-checkins Mailing List for Nagios Open Source

Revision: 2763
          http://nagios.svn.sourceforge.net/nagios/?rev=2763&view=rev
Author:   ageric
Date:     2013-05-08 14:04:10 +0000 (Wed, 08 May 2013)
Log Message:
-----------
xod: Launch a massive strike in the war against strdup()

We already get an allocated and duplicated buffer from mmap_fgets(),
so there's no need for us copy it again, and then once more to get
the value part of the variable in a separate string.

Instead, we avoid the two extra copies per object config entry and
simply modify the input in-place.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/xdata/xodtemplate.c

Modified: nagioscore/trunk/xdata/xodtemplate.c
===================================================================
--- nagioscore/trunk/xdata/xodtemplate.c	2013-05-08 14:03:50 UTC (rev 2762)
+++ nagioscore/trunk/xdata/xodtemplate.c	2013-05-08 14:04:10 UTC (rev 2763)
@@ -1097,9 +1097,7 @@
 	xodtemplate_hostescalation *temp_hostescalation = NULL;
 	xodtemplate_hostextinfo *temp_hostextinfo = NULL;
 	xodtemplate_serviceextinfo *temp_serviceextinfo = NULL;
-	register int x = 0;
-	register int y = 0;
-	int force_skiplists = FALSE;
+	int x, y, force_skiplists = FALSE;
 
 
 	/* should some object definitions be added to skiplists immediately? */
@@ -1174,24 +1172,23 @@
 		}
 
 	/* get variable name */
-	if((variable = (char *)strdup(input)) == NULL)
-		return ERROR;
+	variable = input;
 	/* trim at first whitespace occurance */
-	for(x = 0, y = 0; variable[x] != '\x0'; x++) {
-		if(variable[x] == ' ' || variable[x] == '\t')
+	for(x = 0; variable[x] != '\x0'; x++) {
+		if(variable[x] == ' ' || variable[x] == '\t') {
+			variable[x] = 0;
+			y = x;
 			break;
-		y++;
+			}
 		}
-	variable[y] = '\x0';
 
 	/* get variable value */
-	if((value = (char *)strdup(input + x)) == NULL) {
-		my_free(variable);
-		return ERROR;
-		}
+	value = input + x + 1;
+	while (*value == ' ' || *value == '\t')
+		value++;
+	/* now strip trailing spaces */
 	strip(value);
 
-
 	switch(xodtemplate_current_object_type) {
 
 		case XODTEMPLATE_TIMEPERIOD:
@@ -3466,10 +3463,6 @@
 			break;
 		}
 
-	/* free memory */
-	my_free(variable);
-	my_free(value);
-
 	return result;
 	}
 

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2762
          http://nagios.svn.sourceforge.net/nagios/?rev=2762&view=rev
Author:   ageric
Date:     2013-05-08 14:03:50 +0000 (Wed, 08 May 2013)
Log Message:
-----------
core: free() more variables when quitting

While we'd like to get "No leaks possible" from valgrind, we'll
have to settle for the memory my_strtok() allocates and keeps a
pointer to (for now anyways).

We also make sure to assign more of the "runtime constant" macros
from our global variables, to avoid tracking both of them for leaks.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/base/config.c
    nagioscore/trunk/base/utils.c

Modified: nagioscore/trunk/base/config.c
===================================================================
--- nagioscore/trunk/base/config.c	2013-05-08 14:03:29 UTC (rev 2761)
+++ nagioscore/trunk/base/config.c	2013-05-08 14:03:50 UTC (rev 2762)
@@ -227,8 +227,7 @@
 			command_file = nspath_absolute(value, config_file_dir);
 
 			/* save the macro */
-			my_free(mac->x[MACRO_COMMANDFILE]);
-			mac->x[MACRO_COMMANDFILE] = (char *)strdup(value);
+			mac->x[MACRO_COMMANDFILE] = command_file;
 			}
 
 		else if(!strcmp(variable, "temp_file")) {
@@ -1188,10 +1187,8 @@
 		}
 
 	/* save the macros */
-	my_free(mac->x[MACRO_TEMPPATH]);
-	my_free(mac->x[MACRO_TEMPFILE]);
-	mac->x[MACRO_TEMPPATH] = (char *)strdup(temp_path);
-	mac->x[MACRO_TEMPFILE] = (char *)strdup(temp_file);
+	mac->x[MACRO_TEMPPATH] = temp_path;
+	mac->x[MACRO_TEMPFILE] = temp_file;
 
 	/* adjust timezone values */
 	if(use_timezone != NULL)

Modified: nagioscore/trunk/base/utils.c
===================================================================
--- nagioscore/trunk/base/utils.c	2013-05-08 14:03:29 UTC (rev 2761)
+++ nagioscore/trunk/base/utils.c	2013-05-08 14:03:50 UTC (rev 2762)
@@ -3099,6 +3099,8 @@
 
 /* free the memory allocated to the linked lists */
 void free_memory(nagios_macros *mac) {
+	int i;
+
 	/* free all allocated memory for the object definitions */
 	free_object_data();
 
@@ -3148,15 +3150,30 @@
 	my_free(new_program_version);
 
 	/* free file/path variables */
+	my_free(debug_file);
 	my_free(log_file);
 	mac->x[MACRO_LOGFILE] = NULL; /* assigned from 'log_file' */
-	my_free(debug_file);
 	my_free(temp_file);
+	mac->x[MACRO_TEMPFILE] = NULL; /* assigned from temp_file */
 	my_free(temp_path);
+	mac->x[MACRO_TEMPPATH] = NULL; /*assigned from temp_path */
 	my_free(check_result_path);
 	my_free(command_file);
+	mac->x[MACRO_COMMANDFILE] = NULL; /* assigned from command_file */
 	my_free(log_archive_path);
+	my_free(lock_file);
 
+	for (i = 0; i < MAX_USER_MACROS; i++) {
+		my_free(macro_user[i]);
+	}
+
+	/* these have no other reference */
+	my_free(mac->x[MACRO_ADMINEMAIL]);
+	my_free(mac->x[MACRO_ADMINPAGER]);
+	my_free(mac->x[MACRO_RESOURCEFILE]);
+	my_free(mac->x[MACRO_OBJECTCACHEFILE]);
+	my_free(mac->x[MACRO_MAINCONFIGFILE]);
+
 	return;
 	}
 

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2761
          http://nagios.svn.sourceforge.net/nagios/?rev=2761&view=rev
Author:   ageric
Date:     2013-05-08 14:03:29 +0000 (Wed, 08 May 2013)
Log Message:
-----------
xod: Avoid printf() calls

Changing it to a logging call makes a lot more sense.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/xdata/xodtemplate.c

Modified: nagioscore/trunk/xdata/xodtemplate.c
===================================================================
--- nagioscore/trunk/xdata/xodtemplate.c	2013-05-08 14:03:12 UTC (rev 2760)
+++ nagioscore/trunk/xdata/xodtemplate.c	2013-05-08 14:03:29 UTC (rev 2761)
@@ -8973,7 +8973,7 @@
 		/* expand host */
 		result = xodtemplate_expand_hostgroups(&glist, reject, hostgroups, _config_file, _start_line);
 		if(result != OK) {
-			printf("Failed to expand hostgroups '%s' to something sensible\n", hostgroups);
+			logit(NSLOG_CONFIG_ERROR, TRUE, "Failed to expand hostgroups '%s' to something sensible\n", hostgroups);
 			free_objectlist(&glist);
 			bitmap_destroy(reject);
 			return NULL;

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2760
          http://nagios.svn.sourceforge.net/nagios/?rev=2760&view=rev
Author:   ageric
Date:     2013-05-08 14:03:12 +0000 (Wed, 08 May 2013)
Log Message:
-----------
core: Make valgrind report leaked and reachable memory more accurately

Mostly for the --verify-config case though, where we have to free
the nebmod lists and some miscellaneous variables.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/base/nagios.c

Modified: nagioscore/trunk/base/nagios.c
===================================================================
--- nagioscore/trunk/base/nagios.c	2013-05-08 14:02:54 UTC (rev 2759)
+++ nagioscore/trunk/base/nagios.c	2013-05-08 14:03:12 UTC (rev 2760)
@@ -509,6 +509,12 @@
 
 		/* exit */
 		timing_point("Exiting\n");
+
+		/* make valgrind shut up about still reachable memory */
+		neb_free_module_list();
+		free(config_file_dir);
+		free(config_file);
+
 		exit(result);
 		}
 
@@ -846,6 +852,8 @@
 		/* free misc memory */
 		my_free(lock_file);
 		my_free(config_file);
+		my_free(config_file_dir);
+		my_free(nagios_binary_path);
 		}
 
 	return OK;

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2759
          http://nagios.svn.sourceforge.net/nagios/?rev=2759&view=rev
Author:   ageric
Date:     2013-05-08 14:02:54 +0000 (Wed, 08 May 2013)
Log Message:
-----------
core: Stop leaking neb-modules on reloads and shutdowns

Only 40-ish bytes per module, but it's good to get valgrind to
shut the hell up.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/base/nebmods.c

Modified: nagioscore/trunk/base/nebmods.c
===================================================================
--- nagioscore/trunk/base/nebmods.c	2013-05-08 14:02:36 UTC (rev 2758)
+++ nagioscore/trunk/base/nebmods.c	2013-05-08 14:02:54 UTC (rev 2759)
@@ -129,6 +129,7 @@
 			continue;
 		my_free(temp_module->filename);
 		my_free(temp_module->args);
+		my_free(temp_module);
 		}
 
 	neb_module_list = NULL;

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2758
          http://nagios.svn.sourceforge.net/nagios/?rev=2758&view=rev
Author:   ageric
Date:     2013-05-08 14:02:36 +0000 (Wed, 08 May 2013)
Log Message:
-----------
core: Assign strings rather than copying them when registering objects

This means we avoid one layer of strdup(). Since everything is
already copied from config to pre-objects anyway, and then from
templates to pre-objects, we really needn't bother with it when
creating the real objects too.

This cuts down the number of memory allocations by quite a significant
amount. We still copy everything from services though, since those
are duplicated per-object, and releasing duplicated services would
otherwise lead to heap corruption on double-free when we exit or
reload.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/common/objects.c
    nagioscore/trunk/xdata/xodtemplate.c

Modified: nagioscore/trunk/common/objects.c
===================================================================
--- nagioscore/trunk/common/objects.c	2013-05-08 14:02:12 UTC (rev 2757)
+++ nagioscore/trunk/common/objects.c	2013-05-08 14:02:36 UTC (rev 2758)
@@ -421,10 +421,8 @@
 		return NULL;
 
 	/* copy string vars */
-	if((new_timeperiod->name = (char *)strdup(name)) == NULL)
-		return NULL;
-	if((new_timeperiod->alias = (char *)strdup(alias)) == NULL)
-		result = ERROR;
+	new_timeperiod->name = name;
+	new_timeperiod->alias = alias ? alias : name;
 
 	/* add new timeperiod to hash table */
 	if(result == OK) {
@@ -446,8 +444,7 @@
 
 	/* handle errors */
 	if(result == ERROR) {
-		my_free(new_timeperiod->alias);
-		my_free(new_timeperiod->name);
+		free(new_timeperiod);
 		return NULL;
 		}
 
@@ -624,7 +621,7 @@
 		return NULL;
 	}
 	if(notification_period && !(notify_tp = find_timeperiod(notification_period))) {
-		logit(NSLOG_CONFIG_ERROR, TRUE, "Error: Failed to locate noticiation_period '%s' for host '%s'!\n",
+		logit(NSLOG_CONFIG_ERROR, TRUE, "Error: Failed to locate notification_period '%s' for host '%s'!\n",
 			  notification_period, name);
 		return NULL;
 	}
@@ -652,57 +649,25 @@
 
 	new_host = calloc(1, sizeof(*new_host));
 
-	/* duplicate string vars */
-	if((new_host->name = (char *)strdup(name)) == NULL)
-		return NULL;
-	new_host->display_name = display_name ? strdup(display_name) : new_host->name;
-	new_host->alias = alias ? strdup(alias) : new_host->name;
-	new_host->address = address ? strdup(address) : new_host->name;
-	if(!new_host->display_name || !new_host->alias || !new_host->address)
-		result = ERROR;
+	/* assign string vars */
+	new_host->name = name;
+	new_host->display_name = display_name ? display_name : new_host->name;
+	new_host->alias = alias ? alias : new_host->name;
+	new_host->address = address ? address : new_host->name;
 	new_host->check_period = check_tp ? check_tp->name : NULL;
 	new_host->notification_period = notify_tp ? notify_tp->name : NULL;
 	new_host->notification_period_ptr = notify_tp;
 	new_host->check_period_ptr = check_tp;
+	new_host->check_command = check_command;
+	new_host->event_handler = event_handler;
+	new_host->notes = notes;
+	new_host->notes_url = notes_url;
+	new_host->action_url = action_url;
+	new_host->icon_image = icon_image;
+	new_host->icon_image_alt = icon_image_alt;
+	new_host->vrml_image = vrml_image;
+	new_host->statusmap_image = statusmap_image;
 
-	if(check_command) {
-		if((new_host->check_command = (char *)strdup(check_command)) == NULL)
-			result = ERROR;
-		}
-	if(event_handler) {
-		if((new_host->event_handler = (char *)strdup(event_handler)) == NULL)
-			result = ERROR;
-		}
-	if(notes) {
-		if((new_host->notes = (char *)strdup(notes)) == NULL)
-			result = ERROR;
-		}
-	if(notes_url) {
-		if((new_host->notes_url = (char *)strdup(notes_url)) == NULL)
-			result = ERROR;
-		}
-	if(action_url) {
-		if((new_host->action_url = (char *)strdup(action_url)) == NULL)
-			result = ERROR;
-		}
-	if(icon_image) {
-		if((new_host->icon_image = (char *)strdup(icon_image)) == NULL)
-			result = ERROR;
-		}
-	if(icon_image_alt) {
-		if((new_host->icon_image_alt = (char *)strdup(icon_image_alt)) == NULL)
-			result = ERROR;
-		}
-	if(vrml_image) {
-		if((new_host->vrml_image = (char *)strdup(vrml_image)) == NULL)
-			result = ERROR;
-		}
-	if(statusmap_image) {
-		if((new_host->statusmap_image = (char *)strdup(statusmap_image)) == NULL)
-			result = ERROR;
-		}
-
-
 	/* duplicate non-string vars */
 	new_host->hourly_value = hourly_value;
 	new_host->max_attempts = max_attempts;
@@ -768,25 +733,7 @@
 
 	/* handle errors */
 	if(result == ERROR) {
-#ifdef NSCORE
-		my_free(new_host->plugin_output);
-		my_free(new_host->long_plugin_output);
-		my_free(new_host->perf_data);
-#endif
-		my_free(new_host->statusmap_image);
-		my_free(new_host->vrml_image);
-		my_free(new_host->icon_image_alt);
-		my_free(new_host->icon_image);
-		my_free(new_host->action_url);
-		my_free(new_host->notes_url);
-		my_free(new_host->notes);
-		my_free(new_host->event_handler);
-		my_free(new_host->check_command);
-		my_free(new_host->address);
-		my_free(new_host->alias);
-		if(display_name)
-			my_free(new_host->display_name);
-		my_free(new_host->name);
+		my_free(new_host);
 		return NULL;
 		}
 
@@ -971,23 +918,12 @@
 
 	new_hostgroup = calloc(1, sizeof(*new_hostgroup));
 
-	/* duplicate vars */
-	if((new_hostgroup->group_name = (char *)strdup(name)) == NULL)
-		result = ERROR;
-	if((new_hostgroup->alias = (char *)strdup((alias == NULL) ? name : alias)) == NULL)
-		result = ERROR;
-	if(notes) {
-		if((new_hostgroup->notes = (char *)strdup(notes)) == NULL)
-			result = ERROR;
-		}
-	if(notes_url) {
-		if((new_hostgroup->notes_url = (char *)strdup(notes_url)) == NULL)
-			result = ERROR;
-		}
-	if(action_url) {
-		if((new_hostgroup->action_url = (char *)strdup(action_url)) == NULL)
-			result = ERROR;
-		}
+	/* assign vars */
+	new_hostgroup->group_name = name;
+	new_hostgroup->alias = alias ? alias : name;
+	new_hostgroup->notes = notes;
+	new_hostgroup->notes_url = notes_url;
+	new_hostgroup->action_url = action_url;
 
 	/* add new host group to hash table */
 	if(result == OK) {
@@ -1009,8 +945,7 @@
 
 	/* handle errors */
 	if(result == ERROR) {
-		my_free(new_hostgroup->alias);
-		my_free(new_hostgroup->group_name);
+		free(new_hostgroup);
 		return NULL;
 		}
 
@@ -1097,22 +1032,11 @@
 	new_servicegroup = calloc(1, sizeof(*new_servicegroup));
 
 	/* duplicate vars */
-	if((new_servicegroup->group_name = (char *)strdup(name)) == NULL)
-		result = ERROR;
-	if((new_servicegroup->alias = (char *)strdup((alias == NULL) ? name : alias)) == NULL)
-		result = ERROR;
-	if(notes) {
-		if((new_servicegroup->notes = (char *)strdup(notes)) == NULL)
-			result = ERROR;
-		}
-	if(notes_url) {
-		if((new_servicegroup->notes_url = (char *)strdup(notes_url)) == NULL)
-			result = ERROR;
-		}
-	if(action_url) {
-		if((new_servicegroup->action_url = (char *)strdup(action_url)) == NULL)
-			result = ERROR;
-		}
+	new_servicegroup->group_name = name;
+	new_servicegroup->alias = alias ? alias : name;
+	new_servicegroup->notes = notes;
+	new_servicegroup->notes_url = notes_url;
+	new_servicegroup->action_url = action_url;
 
 	/* add new service group to hash table */
 	if(result == OK) {
@@ -1134,8 +1058,7 @@
 
 	/* handle errors */
 	if(result == ERROR) {
-		my_free(new_servicegroup->alias);
-		my_free(new_servicegroup->group_name);
+		my_free(new_servicegroup);
 		return NULL;
 		}
 
@@ -1256,25 +1179,13 @@
 	new_contact->service_notification_period = stp ? stp->name : NULL;
 	new_contact->host_notification_period_ptr = htp;
 	new_contact->service_notification_period_ptr = stp;
-	if((new_contact->name = (char *)strdup(name)) == NULL)
-		result = ERROR;
-	if((new_contact->alias = (char *)strdup((alias == NULL) ? name : alias)) == NULL)
-		result = ERROR;
-	if(email) {
-		if((new_contact->email = (char *)strdup(email)) == NULL)
-			result = ERROR;
-		}
-	if(pager) {
-		if((new_contact->pager = (char *)strdup(pager)) == NULL)
-			result = ERROR;
-		}
+	new_contact->name = name;
+	new_contact->alias = alias ? alias : name;
+	new_contact->email = email;
+	new_contact->pager = pager;
 	if(addresses) {
-		for(x = 0; x < MAX_CONTACT_ADDRESSES; x++) {
-			if(addresses[x]) {
-				if((new_contact->address[x] = (char *)strdup(addresses[x])) == NULL)
-					result = ERROR;
-				}
-			}
+		for(x = 0; x < MAX_CONTACT_ADDRESSES; x++)
+			new_contact->address[x] = addresses[x];
 		}
 
 	new_contact->minimum_value = minimum_value;
@@ -1306,12 +1217,7 @@
 
 	/* handle errors */
 	if(result == ERROR) {
-		for(x = 0; x < MAX_CONTACT_ADDRESSES; x++)
-			my_free(new_contact->address[x]);
-		my_free(new_contact->name);
-		my_free(new_contact->alias);
-		my_free(new_contact->email);
-		my_free(new_contact->pager);
+		free(new_contact);
 		return NULL;
 		}
 
@@ -1417,11 +1323,9 @@
 	if(!new_contactgroup)
 		return NULL;
 
-	/* duplicate vars */
-	if((new_contactgroup->group_name = (char *)strdup(name)) == NULL)
-		result = ERROR;
-	if((new_contactgroup->alias = (char *)strdup((alias == NULL) ? name : alias)) == NULL)
-		result = ERROR;
+	/* assign vars */
+	new_contactgroup->group_name = name;
+	new_contactgroup->alias = alias ? alias : name;
 
 	/* add new contact group to hash table */
 	if(result == OK) {
@@ -1443,8 +1347,7 @@
 
 	/* handle errors */
 	if(result == ERROR) {
-		my_free(new_contactgroup->alias);
-		my_free(new_contactgroup->group_name);
+		free(new_contactgroup);
 		return NULL;
 		}
 
@@ -1698,11 +1601,9 @@
 	if(!new_command)
 		return NULL;
 
-	/* duplicate vars */
-	if((new_command->name = (char *)strdup(name)) == NULL)
-		return NULL;
-	if((new_command->command_line = (char *)strdup(value)) == NULL)
-		result = ERROR;
+	/* assign vars */
+	new_command->name = name;
+	new_command->command_line = value;
 
 	/* add new command to hash table */
 	if(result == OK) {
@@ -1724,8 +1625,7 @@
 
 	/* handle errors */
 	if(result == ERROR) {
-		my_free(new_command->command_line);
-		my_free(new_command->name);
+		my_free(new_command);
 		return NULL;
 		}
 
@@ -2522,8 +2422,9 @@
 			my_free(this_timeperiodexclusion);
 			}
 
+		if (this_timeperiod->alias != this_timeperiod->name)
+			my_free(this_timeperiod->alias);
 		my_free(this_timeperiod->name);
-		my_free(this_timeperiod->alias);
 		my_free(this_timeperiod);
 		}
 
@@ -2630,8 +2531,9 @@
 			this_hostsmember = next_hostsmember;
 			}
 
+		if (this_hostgroup->alias != this_hostgroup->group_name)
+			my_free(this_hostgroup->alias);
 		my_free(this_hostgroup->group_name);
-		my_free(this_hostgroup->alias);
 		my_free(this_hostgroup->notes);
 		my_free(this_hostgroup->notes_url);
 		my_free(this_hostgroup->action_url);
@@ -2653,8 +2555,9 @@
 			this_servicesmember = next_servicesmember;
 			}
 
+		if (this_servicegroup->alias != this_servicegroup->group_name)
+			my_free(this_servicegroup->alias);
 		my_free(this_servicegroup->group_name);
-		my_free(this_servicegroup->alias);
 		my_free(this_servicegroup->notes);
 		my_free(this_servicegroup->notes_url);
 		my_free(this_servicegroup->action_url);
@@ -2699,8 +2602,9 @@
 			this_customvariablesmember = next_customvariablesmember;
 			}
 
+		if (this_contact->alias != this_contact->name)
+			my_free(this_contact->alias);
 		my_free(this_contact->name);
-		my_free(this_contact->alias);
 		my_free(this_contact->email);
 		my_free(this_contact->pager);
 		for(j = 0; j < MAX_CONTACT_ADDRESSES; j++)
@@ -2726,8 +2630,9 @@
 			this_contactsmember = next_contactsmember;
 			}
 
+		if (this_contactgroup->alias != this_contactgroup->group_name)
+			my_free(this_contactgroup->alias);
 		my_free(this_contactgroup->group_name);
-		my_free(this_contactgroup->alias);
 		my_free(this_contactgroup);
 		}
 

Modified: nagioscore/trunk/xdata/xodtemplate.c
===================================================================
--- nagioscore/trunk/xdata/xodtemplate.c	2013-05-08 14:02:12 UTC (rev 2757)
+++ nagioscore/trunk/xdata/xodtemplate.c	2013-05-08 14:02:36 UTC (rev 2758)
@@ -8466,8 +8466,10 @@
 		next_timeperiod = this_timeperiod->next;
 		my_free(this_timeperiod->template);
 		my_free(this_timeperiod->name);
-		my_free(this_timeperiod->timeperiod_name);
-		my_free(this_timeperiod->alias);
+		if (!this_timeperiod->register_object) {
+			my_free(this_timeperiod->timeperiod_name);
+			my_free(this_timeperiod->alias);
+		}
 		for(x = 0; x < 7; x++)
 			my_free(this_timeperiod->timeranges[x]);
 		for(x = 0; x < DATERANGE_TYPES; x++) {
@@ -8488,8 +8490,10 @@
 		next_command = this_command->next;
 		my_free(this_command->template);
 		my_free(this_command->name);
-		my_free(this_command->command_name);
-		my_free(this_command->command_line);
+		if (!this_command->register_object) {
+			my_free(this_command->command_name);
+			my_free(this_command->command_line);
+		}
 		my_free(this_command);
 		}
 	xodtemplate_command_list = NULL;
@@ -8500,8 +8504,10 @@
 		next_contactgroup = this_contactgroup->next;
 		my_free(this_contactgroup->template);
 		my_free(this_contactgroup->name);
-		my_free(this_contactgroup->contactgroup_name);
-		my_free(this_contactgroup->alias);
+		if (!this_contactgroup->register_object) {
+			my_free(this_contactgroup->contactgroup_name);
+			my_free(this_contactgroup->alias);
+		}
 		my_free(this_contactgroup->members);
 		my_free(this_contactgroup->contactgroup_members);
 		bitmap_destroy(this_contactgroup->member_map);
@@ -8517,13 +8523,15 @@
 		next_hostgroup = this_hostgroup->next;
 		my_free(this_hostgroup->template);
 		my_free(this_hostgroup->name);
-		my_free(this_hostgroup->hostgroup_name);
-		my_free(this_hostgroup->alias);
+		if (!this_hostgroup->register_object) {
+			my_free(this_hostgroup->hostgroup_name);
+			my_free(this_hostgroup->alias);
+			my_free(this_hostgroup->notes);
+			my_free(this_hostgroup->notes_url);
+			my_free(this_hostgroup->action_url);
+		}
 		my_free(this_hostgroup->members);
 		my_free(this_hostgroup->hostgroup_members);
-		my_free(this_hostgroup->notes);
-		my_free(this_hostgroup->notes_url);
-		my_free(this_hostgroup->action_url);
 		bitmap_destroy(this_hostgroup->member_map);
 		free_objectlist(&this_hostgroup->member_list);
 		free_objectlist(&this_hostgroup->group_list);
@@ -8537,13 +8545,15 @@
 		next_servicegroup = this_servicegroup->next;
 		my_free(this_servicegroup->template);
 		my_free(this_servicegroup->name);
-		my_free(this_servicegroup->servicegroup_name);
-		my_free(this_servicegroup->alias);
+		if (!this_servicegroup->register_object) {
+			my_free(this_servicegroup->servicegroup_name);
+			my_free(this_servicegroup->alias);
+			my_free(this_servicegroup->notes);
+			my_free(this_servicegroup->notes_url);
+			my_free(this_servicegroup->action_url);
+		}
 		my_free(this_servicegroup->members);
 		my_free(this_servicegroup->servicegroup_members);
-		my_free(this_servicegroup->notes);
-		my_free(this_servicegroup->notes_url);
-		my_free(this_servicegroup->action_url);
 		bitmap_destroy(this_servicegroup->member_map);
 		free_objectlist(&this_servicegroup->member_list);
 		free_objectlist(&this_servicegroup->group_list);
@@ -8554,6 +8564,14 @@
 
 	/* free memory allocated to contact list */
 	for(this_contact = xodtemplate_contact_list; this_contact != NULL; this_contact = next_contact) {
+		if (!this_contact->register_object) {
+			my_free(this_contact->contact_name);
+			my_free(this_contact->alias);
+			my_free(this_contact->email);
+			my_free(this_contact->pager);
+			for (x = 0; x < MAX_XODTEMPLATE_CONTACT_ADDRESSES; x++)
+				my_free(this_contact->address[x]);
+		}
 
 		/* free custom variables */
 		this_customvariablesmember = this_contact->custom_variables;
@@ -8568,13 +8586,7 @@
 		next_contact = this_contact->next;
 		my_free(this_contact->template);
 		my_free(this_contact->name);
-		my_free(this_contact->contact_name);
-		my_free(this_contact->alias);
 		my_free(this_contact->contact_groups);
-		my_free(this_contact->email);
-		my_free(this_contact->pager);
-		for(x = 0; x < MAX_XODTEMPLATE_CONTACT_ADDRESSES; x++)
-			my_free(this_contact->address[x]);
 		my_free(this_contact->service_notification_period);
 		my_free(this_contact->service_notification_commands);
 		my_free(this_contact->host_notification_period);
@@ -8586,7 +8598,22 @@
 
 	/* free memory allocated to host list */
 	for(this_host = xodtemplate_host_list; this_host != NULL; this_host = next_host) {
-
+		next_host = this_host->next;
+		if (!this_host->register_object) {
+			my_free(this_host->host_name);
+			my_free(this_host->alias);
+			my_free(this_host->display_name);
+			my_free(this_host->address);
+			my_free(this_host->check_command);
+			my_free(this_host->event_handler);
+			my_free(this_host->notes);
+			my_free(this_host->notes_url);
+			my_free(this_host->action_url);
+			my_free(this_host->icon_image);
+			my_free(this_host->icon_image_alt);
+			my_free(this_host->statusmap_image);
+			my_free(this_host->vrml_image);
+			}
 		/* free custom variables */
 		this_customvariablesmember = this_host->custom_variables;
 		while(this_customvariablesmember != NULL) {
@@ -8597,28 +8624,14 @@
 			this_customvariablesmember = next_customvariablesmember;
 			}
 
-		next_host = this_host->next;
 		my_free(this_host->template);
 		my_free(this_host->name);
-		my_free(this_host->host_name);
-		my_free(this_host->alias);
-		my_free(this_host->display_name);
-		my_free(this_host->address);
 		my_free(this_host->parents);
 		my_free(this_host->host_groups);
-		my_free(this_host->check_command);
 		my_free(this_host->check_period);
-		my_free(this_host->event_handler);
 		my_free(this_host->contact_groups);
 		my_free(this_host->contacts);
 		my_free(this_host->notification_period);
-		my_free(this_host->notes);
-		my_free(this_host->notes_url);
-		my_free(this_host->action_url);
-		my_free(this_host->icon_image);
-		my_free(this_host->icon_image_alt);
-		my_free(this_host->vrml_image);
-		my_free(this_host->statusmap_image);
 		my_free(this_host);
 		}
 	xodtemplate_host_list = NULL;
@@ -8629,7 +8642,7 @@
 		next_service = this_service->next;
 
 		/* free custom variables */
-		if(this_service->is_copy == FALSE) {
+		if(this_service->is_copy == FALSE || !this_service->register_object) {
 			this_customvariablesmember = this_service->custom_variables;
 			while(this_customvariablesmember != NULL) {
 				next_customvariablesmember = this_customvariablesmember->next;

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2757
          http://nagios.svn.sourceforge.net/nagios/?rev=2757&view=rev
Author:   ageric
Date:     2013-05-08 14:02:12 +0000 (Wed, 08 May 2013)
Log Message:
-----------
core: Avoid strdup()'ing group member names

Since we know all string lists are duplicated to kingdom come when
we inherit variable values, read them from config and register
objects, we can at least avoid duplicating them once again here,
cutting down memory used by lots, and allocations even more (making
it far more pleasant to run large configurations through valgrind).

The user-visible feature here is that error messages for non-existant
objects listed as members of some particular group can contain the
location in the configuration where things went south, which is
particularly helpful for larger configurations.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/base/config.c
    nagioscore/trunk/common/objects.c
    nagioscore/trunk/xdata/xodtemplate.c

Modified: nagioscore/trunk/base/config.c
===================================================================
--- nagioscore/trunk/base/config.c	2013-05-08 14:01:47 UTC (rev 2756)
+++ nagioscore/trunk/base/config.c	2013-05-08 14:02:12 UTC (rev 2757)
@@ -1482,14 +1482,12 @@
 	contact *temp_contact = NULL;
 	commandsmember *temp_commandsmember = NULL;
 	contactgroup *temp_contactgroup = NULL;
-	contactsmember *temp_contactsmember = NULL;
 	host *temp_host = NULL;
 	host *temp_host2 = NULL;
 	hostsmember *temp_hostsmember = NULL;
 	servicesmember *sm = NULL;
 	hostgroup *temp_hostgroup = NULL;
 	servicegroup *temp_servicegroup = NULL;
-	servicesmember *temp_servicesmember = NULL;
 	service *temp_service = NULL;
 	command *temp_command = NULL;
 	timeperiod *temp_timeperiod = NULL;
@@ -1711,24 +1709,6 @@
 	if(verify_config)
 		printf("Checking host groups...\n");
 	for(temp_hostgroup = hostgroup_list, total_objects = 0; temp_hostgroup != NULL; temp_hostgroup = temp_hostgroup->next, total_objects++) {
-
-		/* check all group members */
-		for(temp_hostsmember = temp_hostgroup->members; temp_hostsmember != NULL; temp_hostsmember = temp_hostsmember->next) {
-
-			temp_host = find_host(temp_hostsmember->host_name);
-			if(temp_host == NULL) {
-				logit(NSLOG_VERIFICATION_ERROR, TRUE, "Error: Host '%s' specified in host group '%s' is not defined anywhere!", temp_hostsmember->host_name, temp_hostgroup->group_name);
-				errors++;
-				}
-
-			/* save a pointer to this hostgroup for faster host/group membership lookups later */
-			else
-				add_object_to_objectlist(&temp_host->hostgroups_ptr, (void *)temp_hostgroup);
-
-			/* save host pointer for later */
-			temp_hostsmember->host_ptr = temp_host;
-			}
-
 		/* check for illegal characters in hostgroup name */
 		if(use_precached_objects == FALSE) {
 			if(contains_illegal_object_chars(temp_hostgroup->group_name) == TRUE) {
@@ -1748,24 +1728,6 @@
 	if(verify_config)
 		printf("Checking service groups...\n");
 	for(temp_servicegroup = servicegroup_list, total_objects = 0; temp_servicegroup != NULL; temp_servicegroup = temp_servicegroup->next, total_objects++) {
-
-		/* check all group members */
-		for(temp_servicesmember = temp_servicegroup->members; temp_servicesmember != NULL; temp_servicesmember = temp_servicesmember->next) {
-
-			temp_service = find_service(temp_servicesmember->host_name, temp_servicesmember->service_description);
-			if(temp_service == NULL) {
-				logit(NSLOG_VERIFICATION_ERROR, TRUE, "Error: Service '%s' on host '%s' specified in service group '%s' is not defined anywhere!", temp_servicesmember->service_description, temp_servicesmember->host_name, temp_servicegroup->group_name);
-				errors++;
-				}
-
-			/* save a pointer to this servicegroup for faster service/group membership lookups later */
-			else
-				add_object_to_objectlist(&temp_service->servicegroups_ptr, (void *)temp_servicegroup);
-
-			/* save service pointer for later */
-			temp_servicesmember->service_ptr = temp_service;
-			}
-
 		/* check for illegal characters in servicegroup name */
 		if(use_precached_objects == FALSE) {
 			if(contains_illegal_object_chars(temp_servicegroup->group_name) == TRUE) {
@@ -1883,24 +1845,6 @@
 	if(verify_config)
 		printf("Checking contact groups...\n");
 	for(temp_contactgroup = contactgroup_list, total_objects = 0; temp_contactgroup != NULL; temp_contactgroup = temp_contactgroup->next, total_objects++) {
-
-		/* check all the group members */
-		for(temp_contactsmember = temp_contactgroup->members; temp_contactsmember != NULL; temp_contactsmember = temp_contactsmember->next) {
-
-			temp_contact = find_contact(temp_contactsmember->contact_name);
-			if(temp_contact == NULL) {
-				logit(NSLOG_VERIFICATION_ERROR, TRUE, "Error: Contact '%s' specified in contact group '%s' is not defined anywhere!", temp_contactsmember->contact_name, temp_contactgroup->group_name);
-				errors++;
-				}
-
-			/* save a pointer to this contactgroup for faster contact/group membership lookups later */
-			else
-				add_object_to_objectlist(&temp_contact->contactgroups_ptr, (void *)temp_contactgroup);
-
-			/* save the contact pointer for later */
-			temp_contactsmember->contact_ptr = temp_contact;
-			}
-
 		/* check for illegal characters in contactgroup name */
 		if(use_precached_objects == FALSE) {
 			if(contains_illegal_object_chars(temp_contactgroup->group_name) == TRUE) {

Modified: nagioscore/trunk/common/objects.c
===================================================================
--- nagioscore/trunk/common/objects.c	2013-05-08 14:01:47 UTC (rev 2756)
+++ nagioscore/trunk/common/objects.c	2013-05-08 14:02:12 UTC (rev 2757)
@@ -1027,22 +1027,25 @@
 	hostsmember *new_member = NULL;
 	hostsmember *last_member = NULL;
 	hostsmember *temp_member = NULL;
+	struct host *h;
 
 	/* make sure we have the data we need */
 	if(temp_hostgroup == NULL || (host_name == NULL || !strcmp(host_name, ""))) {
 		logit(NSLOG_CONFIG_ERROR, TRUE, "Error: Hostgroup or group member is NULL\n");
 		return NULL;
 		}
+	if (!(h = find_host(host_name))) {
+		logit(NSLOG_CONFIG_ERROR, TRUE, "Error: Failed to locate host '%s' for hostgroup '%s'\n", host_name, temp_hostgroup->group_name);
+		return NULL;
+	}
 
 	/* allocate memory for a new member */
 	if((new_member = calloc(1, sizeof(hostsmember))) == NULL)
 		return NULL;
 
-	/* duplicate vars */
-	if((new_member->host_name = (char *)strdup(host_name)) == NULL) {
-		my_free(new_member);
-		return NULL;
-		}
+	/* assign vars */
+	new_member->host_name = h->name;
+	new_member->host_ptr = h;
 
 	/* add the new member to the member list, sorted by host name */
 #ifndef NSCGI
@@ -1074,6 +1077,8 @@
 		last_member->next = new_member;
 		}
 
+	prepend_object_to_objectlist(&h->hostgroups_ptr, (void *)temp_hostgroup);
+
 	return new_member;
 	}
 
@@ -1147,26 +1152,26 @@
 	servicesmember *new_member = NULL;
 	servicesmember *last_member = NULL;
 	servicesmember *temp_member = NULL;
+	struct service *svc;
 
 	/* make sure we have the data we need */
 	if(temp_servicegroup == NULL || (host_name == NULL || !strcmp(host_name, "")) || (svc_description == NULL || !strcmp(svc_description, ""))) {
 		logit(NSLOG_CONFIG_ERROR, TRUE, "Error: Servicegroup or group member is NULL\n");
 		return NULL;
 		}
+	if (!(svc = find_service(host_name, svc_description))) {
+		logit(NSLOG_CONFIG_ERROR, TRUE, "Error: Failed to locate service '%s' on host '%s' for servicegroup '%s'\n", svc_description, host_name, temp_servicegroup->group_name);
+		return NULL;
+		}
 
 	/* allocate memory for a new member */
 	if((new_member = calloc(1, sizeof(servicesmember))) == NULL)
 		return NULL;
 
-	/* duplicate vars */
-	new_member->host_name = (char *)strdup(host_name);
-	new_member->service_description = (char *)strdup(svc_description);
-	if(new_member->host_name == NULL || new_member->service_description == NULL) {
-		my_free(new_member->host_name);
-		my_free(new_member->service_description);
-		my_free(new_member);
-		return NULL;
-		}
+	/* assign vars */
+	new_member->host_name = svc->host_name;
+	new_member->service_description = svc->description;
+	new_member->service_ptr = svc;
 
 	/*
 	 * add new member to member list, sorted by host name then
@@ -1213,6 +1218,8 @@
 		last_member->next = new_member;
 		}
 
+	prepend_object_to_objectlist(&svc->servicegroups_ptr, (void *)temp_servicegroup);
+
 	return new_member;
 	}
 
@@ -1453,6 +1460,7 @@
 /* add a new member to a contact group */
 contactsmember *add_contact_to_contactgroup(contactgroup *grp, char *contact_name) {
 	contactsmember *new_contactsmember = NULL;
+	struct contact *c;
 
 	/* make sure we have the data we need */
 	if(grp == NULL || (contact_name == NULL || !strcmp(contact_name, ""))) {
@@ -1460,20 +1468,25 @@
 		return NULL;
 		}
 
+	if (!(c = find_contact(contact_name))) {
+		logit(NSLOG_CONFIG_ERROR, TRUE, "Error: Failed to locate contact '%s' for contactgroup '%s'\n", contact_name, grp->group_name);
+		return NULL;
+		}
+
 	/* allocate memory for a new member */
 	if((new_contactsmember = calloc(1, sizeof(contactsmember))) == NULL)
 		return NULL;
 
-	/* duplicate vars */
-	if((new_contactsmember->contact_name = (char *)strdup(contact_name)) == NULL) {
-		my_free(new_contactsmember);
-		return NULL;
-		}
+	/* assign vars */
+	new_contactsmember->contact_name = c->name;
+	new_contactsmember->contact_ptr = c;
 
 	/* add the new member to the head of the member list */
 	new_contactsmember->next = grp->members;
 	grp->members = new_contactsmember;
 
+	prepend_object_to_objectlist(&c->contactgroups_ptr, (void *)grp);
+
 	return new_contactsmember;
 	}
 
@@ -2613,7 +2626,6 @@
 		this_hostsmember = this_hostgroup->members;
 		while(this_hostsmember != NULL) {
 			next_hostsmember = this_hostsmember->next;
-			my_free(this_hostsmember->host_name);
 			my_free(this_hostsmember);
 			this_hostsmember = next_hostsmember;
 			}
@@ -2637,8 +2649,6 @@
 		this_servicesmember = this_servicegroup->members;
 		while(this_servicesmember != NULL) {
 			next_servicesmember = this_servicesmember->next;
-			my_free(this_servicesmember->host_name);
-			my_free(this_servicesmember->service_description);
 			my_free(this_servicesmember);
 			this_servicesmember = next_servicesmember;
 			}
@@ -2712,7 +2722,6 @@
 		this_contactsmember = this_contactgroup->members;
 		while(this_contactsmember != NULL) {
 			next_contactsmember = this_contactsmember->next;
-			my_free(this_contactsmember->contact_name);
 			my_free(this_contactsmember);
 			this_contactsmember = next_contactsmember;
 			}

Modified: nagioscore/trunk/xdata/xodtemplate.c
===================================================================
--- nagioscore/trunk/xdata/xodtemplate.c	2013-05-08 14:01:47 UTC (rev 2756)
+++ nagioscore/trunk/xdata/xodtemplate.c	2013-05-08 14:02:12 UTC (rev 2757)
@@ -6846,6 +6846,71 @@
 /**************** OBJECT REGISTRATION FUNCTIONS *******************/
 /******************************************************************/
 
+static int xodtemplate_register_contactgroup_members(xodtemplate_contactgroup *this_contactgroup)
+{
+	objectlist *list;
+	struct contactgroup *cg;
+	int num_regs = 0;
+
+	if (!this_contactgroup->register_object)
+		return 0;
+
+	cg = find_contactgroup(this_contactgroup->contactgroup_name);
+	for(list = this_contactgroup->member_list; list; list = list->next) {
+		xodtemplate_contact *c = (xodtemplate_contact *)list->object_ptr;
+		if (!add_contact_to_contactgroup(cg, c->contact_name)) {
+			logit(NSLOG_CONFIG_ERROR, TRUE, "Bad member of contactgroup '%s' (config file '%s', starting on line %d)\n", cg->group_name, xodtemplate_config_file_name(this_contactgroup->_config_file), this_contactgroup->_start_line);
+			return -1;
+			}
+		num_regs++;
+		}
+	return num_regs;
+}
+
+static int xodtemplate_register_hostgroup_members(xodtemplate_hostgroup *this_hostgroup)
+{
+	objectlist *list;
+	struct hostgroup *hg;
+	int num_regs = 0;
+
+	if (!this_hostgroup->register_object)
+		return 0;
+
+	hg = find_hostgroup(this_hostgroup->hostgroup_name);
+	for(list = this_hostgroup->member_list; list; list = list->next) {
+		xodtemplate_host *h = (xodtemplate_host *)list->object_ptr;
+		if (!add_host_to_hostgroup(hg, h->host_name)) {
+			logit(NSLOG_CONFIG_ERROR, TRUE, "Error: Bad member of hostgrop '%s' (config file '%s', starting on line %d)\n", hg->group_name, xodtemplate_config_file_name(this_hostgroup->_config_file), this_hostgroup->_start_line);
+			return -1;
+			}
+		num_regs++;
+		}
+	return num_regs;
+	}
+
+static int xodtemplate_register_servicegroup_members(xodtemplate_servicegroup *this_servicegroup)
+{
+	objectlist *list, *next;
+	struct servicegroup *sg;
+	int num_regs = 0;
+
+	if (!this_servicegroup->register_object)
+		return 0;
+
+	sg = find_servicegroup(this_servicegroup->servicegroup_name);
+	for(list = this_servicegroup->member_list; list; list = next) {
+		xodtemplate_service *s = (xodtemplate_service *)list->object_ptr;
+		next = list->next;
+		if (!add_service_to_servicegroup(sg, s->host_name, s->service_description)) {
+			logit(NSLOG_CONFIG_ERROR, TRUE, "Error: Bad member of servicegroup '%s' (config file '%s', starting on line %d)\n", sg->group_name, xodtemplate_config_file_name(this_servicegroup->_config_file), this_servicegroup->_start_line);
+			return -1;
+			}
+		num_regs++;
+		}
+
+	return num_regs;
+	}
+
 /*
  * registers object definitions
  * The order goes like this:
@@ -6869,7 +6934,7 @@
  */
 int xodtemplate_register_objects(void) {
 	unsigned int i;
-	int result = OK;
+	int mcount;
 	xodtemplate_timeperiod *temp_timeperiod = NULL;
 	xodtemplate_command *temp_command = NULL;
 	xodtemplate_contactgroup *temp_contactgroup = NULL;
@@ -6884,6 +6949,7 @@
 	xodtemplate_servicedependency *sd, *next_sd;
 	xodtemplate_serviceescalation *se, *next_se;
 	unsigned int ocount[NUM_OBJECT_SKIPLISTS];
+	unsigned int tot_members = 0;
 
 
 	for (i = 0; i < ARRAY_SIZE(ocount); i++) {
@@ -6904,7 +6970,7 @@
 	/* register timeperiods */
 	ptr = NULL;
 	for(temp_timeperiod = (xodtemplate_timeperiod *)skiplist_get_first(xobject_skiplists[TIMEPERIOD_SKIPLIST], &ptr); temp_timeperiod != NULL; temp_timeperiod = (xodtemplate_timeperiod *)skiplist_get_next(&ptr)) {
-		if((result = xodtemplate_register_timeperiod(temp_timeperiod)) == ERROR)
+		if(xodtemplate_register_timeperiod(temp_timeperiod) == ERROR)
 			return ERROR;
 		}
 	timing_point("%u timeperiods registered\n", num_objects.timeperiods);
@@ -6912,7 +6978,7 @@
 	/* register commands */
 	ptr = NULL;
 	for(temp_command = (xodtemplate_command *)skiplist_get_first(xobject_skiplists[COMMAND_SKIPLIST], &ptr); temp_command != NULL; temp_command = (xodtemplate_command *)skiplist_get_next(&ptr)) {
-		if((result = xodtemplate_register_command(temp_command)) == ERROR)
+		if(xodtemplate_register_command(temp_command) == ERROR)
 			return ERROR;
 		}
 	timing_point("%u commands registered\n", num_objects.commands);
@@ -6920,7 +6986,7 @@
 	/* register contactgroups */
 	ptr = NULL;
 	for(temp_contactgroup = (xodtemplate_contactgroup *)skiplist_get_first(xobject_skiplists[CONTACTGROUP_SKIPLIST], &ptr); temp_contactgroup != NULL; temp_contactgroup = (xodtemplate_contactgroup *)skiplist_get_next(&ptr)) {
-		if((result = xodtemplate_register_contactgroup(temp_contactgroup)) == ERROR)
+		if(xodtemplate_register_contactgroup(temp_contactgroup) == ERROR)
 			return ERROR;
 		}
 	timing_point("%u contactgroups registered\n", num_objects.contactgroups);
@@ -6928,7 +6994,7 @@
 	/* register hostgroups */
 	ptr = NULL;
 	for(temp_hostgroup = (xodtemplate_hostgroup *)skiplist_get_first(xobject_skiplists[HOSTGROUP_SKIPLIST], &ptr); temp_hostgroup != NULL; temp_hostgroup = (xodtemplate_hostgroup *)skiplist_get_next(&ptr)) {
-		if((result = xodtemplate_register_hostgroup(temp_hostgroup)) == ERROR)
+		if(xodtemplate_register_hostgroup(temp_hostgroup) == ERROR)
 			return ERROR;
 		}
 	timing_point("%u hostgroups registered\n", num_objects.hostgroups);
@@ -6936,7 +7002,7 @@
 	/* register servicegroups */
 	ptr = NULL;
 	for(temp_servicegroup = (xodtemplate_servicegroup *)skiplist_get_first(xobject_skiplists[SERVICEGROUP_SKIPLIST], &ptr); temp_servicegroup != NULL; temp_servicegroup = (xodtemplate_servicegroup *)skiplist_get_next(&ptr)) {
-		if((result = xodtemplate_register_servicegroup(temp_servicegroup)) == ERROR)
+		if(xodtemplate_register_servicegroup(temp_servicegroup) == ERROR)
 			return ERROR;
 		}
 	timing_point("%u servicegroups registered\n", num_objects.servicegroups);
@@ -6944,7 +7010,7 @@
 	/* register contacts */
 	ptr = NULL;
 	for(temp_contact = (xodtemplate_contact *)skiplist_get_first(xobject_skiplists[CONTACT_SKIPLIST], &ptr); temp_contact != NULL; temp_contact = (xodtemplate_contact *)skiplist_get_next(&ptr)) {
-		if((result = xodtemplate_register_contact(temp_contact)) == ERROR)
+		if(xodtemplate_register_contact(temp_contact) == ERROR)
 			return ERROR;
 		}
 	timing_point("%u contacts registered\n", num_objects.contacts);
@@ -6952,7 +7018,7 @@
 	/* register hosts */
 	ptr = NULL;
 	for(temp_host = (xodtemplate_host *)skiplist_get_first(xobject_skiplists[HOST_SKIPLIST], &ptr); temp_host != NULL; temp_host = (xodtemplate_host *)skiplist_get_next(&ptr)) {
-		if((result = xodtemplate_register_host(temp_host)) == ERROR)
+		if(xodtemplate_register_host(temp_host) == ERROR)
 			return ERROR;
 		}
 	timing_point("%u hosts registered\n", num_objects.hosts);
@@ -6960,11 +7026,39 @@
 	/* register services */
 	ptr = NULL;
 	for(temp_service = (xodtemplate_service *)skiplist_get_first(xobject_skiplists[SERVICE_SKIPLIST], &ptr); temp_service != NULL; temp_service = (xodtemplate_service *)skiplist_get_next(&ptr)) {
-		if((result = xodtemplate_register_service(temp_service)) == ERROR)
+		if(xodtemplate_register_service(temp_service) == ERROR)
 			return ERROR;
 		}
 	timing_point("%u services registered\n", num_objects.services);
 
+	/* groups and objects are registered, so join them up */
+	/* register contactgroup members */
+	ptr = NULL; tot_members = 0;
+	for(temp_contactgroup = (xodtemplate_contactgroup *)skiplist_get_first(xobject_skiplists[CONTACTGROUP_SKIPLIST], &ptr); temp_contactgroup != NULL; temp_contactgroup = (xodtemplate_contactgroup *)skiplist_get_next(&ptr)) {
+		if((mcount = xodtemplate_register_contactgroup_members(temp_contactgroup)) < 0)
+			return ERROR;
+		tot_members += mcount;
+		}
+	timing_point("%u contactgroup memberships registered\n", tot_members);
+
+	/* register hostgroup members */
+	ptr = NULL; tot_members = 0;
+	for(temp_hostgroup = (xodtemplate_hostgroup *)skiplist_get_first(xobject_skiplists[HOSTGROUP_SKIPLIST], &ptr); temp_hostgroup != NULL; temp_hostgroup = (xodtemplate_hostgroup *)skiplist_get_next(&ptr)) {
+		if((mcount = xodtemplate_register_hostgroup_members(temp_hostgroup)) < 0)
+			return ERROR;
+		tot_members += mcount;
+		}
+	timing_point("%u hostgroup memberships registered\n", tot_members);
+
+	/* register servicegroup members */
+	ptr = NULL; tot_members = 0;
+	for(temp_servicegroup = (xodtemplate_servicegroup *)skiplist_get_first(xobject_skiplists[SERVICEGROUP_SKIPLIST], &ptr); temp_servicegroup != NULL; temp_servicegroup = (xodtemplate_servicegroup *)skiplist_get_next(&ptr)) {
+		if((mcount = xodtemplate_register_servicegroup_members(temp_servicegroup)) < 0)
+			return ERROR;
+		tot_members += mcount;
+		}
+	timing_point("%u servicegroup memberships registered\n", tot_members);
+
 	/*
 	 * These aren't in skiplists at all, but it's safe to destroy
 	 * them as we go along, since all dupes are at the head of the list
@@ -7212,11 +7306,9 @@
 	}
 
 
-
 /* registers a contactgroup definition */
 int xodtemplate_register_contactgroup(xodtemplate_contactgroup *this_contactgroup) {
 	contactgroup *new_contactgroup = NULL;
-	objectlist *list;
 
 	/* bail out if we shouldn't register this object */
 	if(this_contactgroup->register_object == FALSE)
@@ -7231,11 +7323,6 @@
 		return ERROR;
 		}
 
-	for(list = this_contactgroup->member_list; list; list = list->next) {
-		xodtemplate_contact *c = (xodtemplate_contact *)list->object_ptr;
-		add_contact_to_contactgroup(new_contactgroup, c->contact_name);
-		}
-
 	return OK;
 	}
 
@@ -7244,7 +7331,6 @@
 /* registers a hostgroup definition */
 int xodtemplate_register_hostgroup(xodtemplate_hostgroup *this_hostgroup) {
 	hostgroup *new_hostgroup = NULL;
-	objectlist *list;
 
 	/* bail out if we shouldn't register this object */
 	if(this_hostgroup->register_object == FALSE)
@@ -7258,20 +7344,13 @@
 		logit(NSLOG_CONFIG_ERROR, TRUE, "Error: Could not register hostgroup (config file '%s', starting on line %d)\n", xodtemplate_config_file_name(this_hostgroup->_config_file), this_hostgroup->_start_line);
 		return ERROR;
 		}
-	for(list = this_hostgroup->member_list; list; list = list->next) {
-		xodtemplate_host *h = (xodtemplate_host *)list->object_ptr;
-		add_host_to_hostgroup(new_hostgroup, h->host_name);
-	}
 
 	return OK;
 	}
 
-
-
 /* registers a servicegroup definition */
 int xodtemplate_register_servicegroup(xodtemplate_servicegroup *this_servicegroup) {
 	servicegroup *new_servicegroup = NULL;
-	objectlist *list, *next;
 
 	/* bail out if we shouldn't register this object */
 	if(this_servicegroup->register_object == FALSE)
@@ -7286,17 +7365,10 @@
 		return ERROR;
 		}
 
-	for(list = this_servicegroup->member_list; list; list = next) {
-		xodtemplate_service *s = (xodtemplate_service *)list->object_ptr;
-		next = list->next;
-		add_service_to_servicegroup(new_servicegroup, s->host_name, s->service_description);
-		}
-
 	return OK;
 	}
 
 
-
 /* registers a servicedependency definition */
 int xodtemplate_register_servicedependency(xodtemplate_servicedependency *this_servicedependency) {
 	servicedependency *new_servicedependency = NULL;

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2756
          http://nagios.svn.sourceforge.net/nagios/?rev=2756&view=rev
Author:   ageric
Date:     2013-05-08 14:01:47 +0000 (Wed, 08 May 2013)
Log Message:
-----------
xod: Make sure we free servicedependency templates

Apparently we forgot to do that earlier. For host dependencies this
doesn't seem to be a problem.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/xdata/xodtemplate.c

Modified: nagioscore/trunk/xdata/xodtemplate.c
===================================================================
--- nagioscore/trunk/xdata/xodtemplate.c	2013-05-08 14:01:27 UTC (rev 2755)
+++ nagioscore/trunk/xdata/xodtemplate.c	2013-05-08 14:01:47 UTC (rev 2756)
@@ -4769,9 +4769,16 @@
 	my_free(temp_servicedependency->name);
 	my_free(temp_servicedependency->template);
 
-	/* skip templates */
-	if(temp_servicedependency->register_object == 0)
+	/* skip templates, but free them first */
+	if(temp_servicedependency->register_object == 0) {
+		my_free(temp_servicedependency->host_name);
+		my_free(temp_servicedependency->service_description);
+		my_free(temp_servicedependency->hostgroup_name);
+		my_free(temp_servicedependency->dependent_host_name);
+		my_free(temp_servicedependency->dependent_service_description);
+		my_free(temp_servicedependency->dependent_hostgroup_name);
 		return OK;
+		}
 
 	if(!temp_servicedependency->host_name && !temp_servicedependency->hostgroup_name
 	   && !temp_servicedependency->servicegroup_name)

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2755
          http://nagios.svn.sourceforge.net/nagios/?rev=2755&view=rev
Author:   ageric
Date:     2013-05-08 14:01:27 +0000 (Wed, 08 May 2013)
Log Message:
-----------
xod: Plug a memory leak in xdata/xodtemplate.c

On soft reloads, we used to leak the bitmaps we created to track
contacts, hosts and services. This patch adds the necessary calls
to get rid of it.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/xdata/xodtemplate.c

Modified: nagioscore/trunk/xdata/xodtemplate.c
===================================================================
--- nagioscore/trunk/xdata/xodtemplate.c	2013-05-06 12:18:46 UTC (rev 2754)
+++ nagioscore/trunk/xdata/xodtemplate.c	2013-05-08 14:01:27 UTC (rev 2755)
@@ -502,6 +502,10 @@
 		}
 #endif
 
+	bitmap_destroy(contact_map);
+	bitmap_destroy(host_map);
+	bitmap_destroy(service_map);
+
 	return result;
 	}
 

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2754
          http://nagios.svn.sourceforge.net/nagios/?rev=2754&view=rev
Author:   estanley375
Date:     2013-05-06 12:18:46 +0000 (Mon, 06 May 2013)
Log Message:
-----------
Fixed bug #407: Reloading nagios config causes spaces in notifications to become plus signs (Alexey Dvoryanchikov)

Also fixed bug #444: Nagios 3.5.0 problem with macro $ADMINEMAIL$ : @ is converted to %40 after 2 nagios reload (Duplicate of bug #407)

Modified Paths:
--------------
    nagioscore/branches/nagios-3-4-x/Changelog
    nagioscore/branches/nagios-3-4-x/common/macros.c

Modified: nagioscore/branches/nagios-3-4-x/Changelog
===================================================================
--- nagioscore/branches/nagios-3-4-x/Changelog	2013-05-06 00:40:40 UTC (rev 2753)
+++ nagioscore/branches/nagios-3-4-x/Changelog	2013-05-06 12:18:46 UTC (rev 2754)
@@ -4,6 +4,8 @@
 
 3.5.1 - xx/xx/xxxx
 ------------------
+* Fixed bug #444: Nagios 3.5.0 problem with macro $ADMINEMAIL$ : @ is converted to %40 after 2 nagios reload (Duplicate of bug #407)
+* Fixed bug #407: Reloading nagios config causes spaces in notifications to become plus signs (Alexey Dvoryanchikov)
 * Fixed bug #445: Adding triggered downtime for child hosts causes a SIGSEGV on restart/reload (Eric Stanley)
 * Fixed bug #375: Freshness expiration never reached and bug #427: freshness threshold doesn't work if it is set long (Scott Wilkerson, Eric Stanley)
 * Fixed bug #432: Downtime scheduled as "Nagios Process" and not the Users name (Sam Lansing, Eric Stanley)

Modified: nagioscore/branches/nagios-3-4-x/common/macros.c
===================================================================
--- nagioscore/branches/nagios-3-4-x/common/macros.c	2013-05-06 00:40:40 UTC (rev 2753)
+++ nagioscore/branches/nagios-3-4-x/common/macros.c	2013-05-06 12:18:46 UTC (rev 2754)
@@ -2585,6 +2585,7 @@
 	for(x = 0; x < MACRO_X_COUNT; x++) {
 		macro_keys[x].code = x;
 		macro_keys[x].name = macro_x_names[x];
+		macro_keys[x].clean_options = 0;
 
 		/* host/service output/perfdata and author/comment macros should get cleaned */
 		if((x >= 16 && x <= 19) || (x >= 49 && x <= 52) || (x >= 99 && x <= 100) || (x >= 124 && x <= 127)) {

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2753
          http://nagios.svn.sourceforge.net/nagios/?rev=2753&view=rev
Author:   estanley375
Date:     2013-05-06 00:40:40 +0000 (Mon, 06 May 2013)
Log Message:
-----------
Forward patch of SVN revision 2752

Revision Links:
--------------
    http://nagios.svn.sourceforge.net/nagios/?rev=2752&view=rev

Modified Paths:
--------------
    nagioscore/trunk/common/downtime.c

Modified: nagioscore/trunk/common/downtime.c
===================================================================
--- nagioscore/trunk/common/downtime.c	2013-05-06 00:38:36 UTC (rev 2752)
+++ nagioscore/trunk/common/downtime.c	2013-05-06 00:40:40 UTC (rev 2753)
@@ -343,21 +343,29 @@
 			}
 		}
 
-		if((FALSE == temp_downtime->fixed) && (FALSE == was_in_effect)) {
-			/* increment pending flex downtime counter */
-			if(temp_downtime->type == HOST_DOWNTIME)
-				hst->pending_flex_downtime++;
-			else
-				svc->pending_flex_downtime++;
-			temp_downtime->incremented_pending_downtime = TRUE;
+	/* If the downtime is triggered and was in effect, mark it as not in 
+		effect so it gets scheduled correctly */
+	if((temp_downtime->triggered_by != 0) && 
+			(TRUE == temp_downtime->is_in_effect)) {
+		was_in_effect = temp_downtime->is_in_effect;
+		temp_downtime->is_in_effect = FALSE;
+		}
 
-			/* Since a flex downtime may never start, schedule an expiring event in
-				case the event is never triggered. The expire event will NOT cancel
-				a downtime event that is in effect */
-			log_debug_info(DEBUGL_DOWNTIME, 1, "Scheduling downtime expire event in case flexible downtime is never triggered\n");
-			temp_downtime->stop_event = schedule_new_event(EVENT_EXPIRE_DOWNTIME, TRUE, (temp_downtime->end_time + 1), FALSE, 0, NULL, FALSE, NULL, NULL, 0);
-			}
+	if((FALSE == temp_downtime->fixed) && (FALSE == was_in_effect)) {
+		/* increment pending flex downtime counter */
+		if(temp_downtime->type == HOST_DOWNTIME)
+			hst->pending_flex_downtime++;
+		else
+			svc->pending_flex_downtime++;
+		temp_downtime->incremented_pending_downtime = TRUE;
 
+		/* Since a flex downtime may never start, schedule an expiring event in
+			case the event is never triggered. The expire event will NOT cancel
+			a downtime event that is in effect */
+		log_debug_info(DEBUGL_DOWNTIME, 1, "Scheduling downtime expire event in case flexible downtime is never triggered\n");
+		temp_downtime->stop_event = schedule_new_event(EVENT_EXPIRE_DOWNTIME, TRUE, (temp_downtime->end_time + 1), FALSE, 0, NULL, FALSE, NULL, NULL, 0);
+		}
+
 #ifdef PROBABLY_NOT_NEEDED
 	/*** FLEXIBLE DOWNTIME SANITY CHECK - ADDED 02/17/2008 ****/
 
@@ -1096,6 +1104,39 @@
 static int downtime_compar(const void *p1, const void *p2) {
 	scheduled_downtime *d1 = *(scheduled_downtime **)p1;
 	scheduled_downtime *d2 = *(scheduled_downtime **)p2;
+
+	/*
+ 		If the start times of two downtimes are equal and one is triggered but
+		but the other is not, the triggered downtime should be later in the
+		list than the untriggered one. This is so they are written to the
+		retention.dat and status.dat in the correct order.
+
+		Previously the triggered downtime always appeared before its 
+		triggering downtime in those files. When the downtimes were read 
+		from those files, either on a core restart or by the CGIs, the 
+		triggered downtime would be discarded because the triggering 
+		downtime did not yet exist.
+
+		The most common case for this is when a downtime is created and 
+		the option is selected to create triggered downtimes on all child 
+		objects. This change in the sort order does NOT resolve the 
+		case where a manually created, triggered downtime is created with 
+		a start time earlier than the triggering downtime.
+
+		This would need to be resolved by comparing the triggered_by value
+		with the downtime ID regardless of the start time. However, this
+		should be a relatively rare case and only caused by intentional
+		scheduling by a human. This change was not implemented because it
+		would cause the downtime list to be out of time order and the
+		implications of this were not well understood.
+	*/
+
+	if(d1->start_time == d2->start_time) {
+		if(( d1->triggered_by == 0 && d2->triggered_by != 0) ||
+				( d1->triggered_by != 0 && d2->triggered_by == 0)) {
+			return d1->triggered_by == 0 ? -1 : 1;
+			}
+		}
 	return (d1->start_time < d2->start_time) ? -1 : (d1->start_time - d2->start_time);
 	}
 

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2752
          http://nagios.svn.sourceforge.net/nagios/?rev=2752&view=rev
Author:   estanley375
Date:     2013-05-06 00:38:36 +0000 (Mon, 06 May 2013)
Log Message:
-----------
Fixed bug #445: Adding triggered downtime for child hosts causes a SIGSEGV on restart/reload

This was caused by triggered downtimes being deleted the when the
triggering downtime was restarted. It was deleted because it was
still marked as in effect. It is now marked as not in effect in the
register_downtime() function.

A related issue, also resolved, is that after a restart, the
triggered downtime was dropped. The same issue also caused the CGI
not to list the triggered downtime. This was due to the ordering
of the downtimes in the retention.dat and status.dat files.

Previously the triggered downtime always appeared before its
triggering downtime in those files. When the downtimes were read
from those files, either on a core restart or by the CGIs, the
triggered downtime would be discarded because the triggering
downtime did not yet exist.

The most common case for this is when a downtime is created and
the option is selected to create triggered downtimes on all child
objects. A change was made in the way downtimes are sorted so that
triggered downtimes with the same start times as untriggered 
downtimes always appear later in the list.  This change in the 
sort order does NOT resolve the case where a manually created, 
triggered downtime is created with a start time earlier than the 
triggering downtime.

This would need to be resolved by comparing the triggered_by value
with the downtime ID regardless of the start time. However, this
should be a relatively rare case and only caused by intentional
scheduling by a human. This change was not implemented because it
would cause the downtime list to be out of time order and the
implications of this were not well understood.

Modified Paths:
--------------
    nagioscore/branches/nagios-3-4-x/Changelog
    nagioscore/branches/nagios-3-4-x/common/downtime.c

Modified: nagioscore/branches/nagios-3-4-x/Changelog
===================================================================
--- nagioscore/branches/nagios-3-4-x/Changelog	2013-04-30 07:45:27 UTC (rev 2751)
+++ nagioscore/branches/nagios-3-4-x/Changelog	2013-05-06 00:38:36 UTC (rev 2752)
@@ -4,6 +4,7 @@
 
 3.5.1 - xx/xx/xxxx
 ------------------
+* Fixed bug #445: Adding triggered downtime for child hosts causes a SIGSEGV on restart/reload (Eric Stanley)
 * Fixed bug #375: Freshness expiration never reached and bug #427: freshness threshold doesn't work if it is set long (Scott Wilkerson, Eric Stanley)
 * Fixed bug #432: Downtime scheduled as "Nagios Process" and not the Users name (Sam Lansing, Eric Stanley)
 

Modified: nagioscore/branches/nagios-3-4-x/common/downtime.c
===================================================================
--- nagioscore/branches/nagios-3-4-x/common/downtime.c	2013-04-30 07:45:27 UTC (rev 2751)
+++ nagioscore/branches/nagios-3-4-x/common/downtime.c	2013-05-06 00:38:36 UTC (rev 2752)
@@ -360,6 +360,14 @@
 			}
 		}
 
+	/* If the downtime is triggered and was in effect, mark it as not in 
+		effect so it gets scheduled correctly */
+	if((temp_downtime->triggered_by != 0) && 
+			(TRUE == temp_downtime->is_in_effect)) {
+		was_in_effect = temp_downtime->is_in_effect;
+		temp_downtime->is_in_effect = FALSE;
+		}
+
 	if((FALSE == temp_downtime->fixed) && (FALSE == was_in_effect)) {
 		/* increment pending flex downtime counter */
 		if(temp_downtime->type == HOST_DOWNTIME)
@@ -1111,6 +1119,39 @@
 static int downtime_compar(const void *p1, const void *p2) {
 	scheduled_downtime *d1 = *(scheduled_downtime **)p1;
 	scheduled_downtime *d2 = *(scheduled_downtime **)p2;
+
+	/*
+ 		If the start times of two downtimes are equal and one is triggered but
+		but the other is not, the triggered downtime should be later in the
+		list than the untriggered one. This is so they are written to the
+		retention.dat and status.dat in the correct order.
+
+		Previously the triggered downtime always appeared before its 
+		triggering downtime in those files. When the downtimes were read 
+		from those files, either on a core restart or by the CGIs, the 
+		triggered downtime would be discarded because the triggering 
+		downtime did not yet exist.
+
+		The most common case for this is when a downtime is created and 
+		the option is selected to create triggered downtimes on all child 
+		objects. This change in the sort order does NOT resolve the 
+		case where a manually created, triggered downtime is created with 
+		a start time earlier than the triggering downtime.
+
+		This would need to be resolved by comparing the triggered_by value
+		with the downtime ID regardless of the start time. However, this
+		should be a relatively rare case and only caused by intentional
+		scheduling by a human. This change was not implemented because it
+		would cause the downtime list to be out of time order and the
+		implications of this were not well understood.
+	*/
+
+	if(d1->start_time == d2->start_time) {
+		if(( d1->triggered_by == 0 && d2->triggered_by != 0) ||
+				( d1->triggered_by != 0 && d2->triggered_by == 0)) {
+			return d1->triggered_by == 0 ? -1 : 1;
+			}
+		}
 	return (d1->start_time < d2->start_time) ? -1 : (d1->start_time - d2->start_time);
 	}
 

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2751
          http://nagios.svn.sourceforge.net/nagios/?rev=2751&view=rev
Author:   ageric
Date:     2013-04-30 07:45:27 +0000 (Tue, 30 Apr 2013)
Log Message:
-----------
core: Fix macro initialization during reloads

When we originally start, the statically defined macro_keys array
is initialized with all zeroes, like the rest of the .bss segment,
but when we get a reload signal, it appears it's sometimes changed,
which caused us to url-encode certain macros that really shouldn't
be url-encoded.

With this patch, we should be able to lay issue 407 to rest for
Nagios 4.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/common/macros.c

Modified: nagioscore/trunk/common/macros.c
===================================================================
--- nagioscore/trunk/common/macros.c	2013-04-22 16:17:33 UTC (rev 2750)
+++ nagioscore/trunk/common/macros.c	2013-04-30 07:45:27 UTC (rev 2751)
@@ -2492,6 +2492,7 @@
 	for (x = 0; x < MACRO_X_COUNT; x++) {
 		macro_keys[x].code = x;
 		macro_keys[x].name = macro_x_names[x];
+		macro_keys[x].clean_options = 0;
 
 		switch (x) {
 			/* output, perfdata, comments and author names need cleaning */

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2750
          http://nagios.svn.sourceforge.net/nagios/?rev=2750&view=rev
Author:   ageric
Date:     2013-04-22 16:17:33 +0000 (Mon, 22 Apr 2013)
Log Message:
-----------
.gitignore lib/test-{fanout,nsutils} and lib/core

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/lib/.gitignore

Modified: nagioscore/trunk/lib/.gitignore
===================================================================
--- nagioscore/trunk/lib/.gitignore	2013-04-16 12:47:26 UTC (rev 2749)
+++ nagioscore/trunk/lib/.gitignore	2013-04-22 16:17:33 UTC (rev 2750)
@@ -5,5 +5,8 @@
 test-bitmap
 test-dkhash
 test-runcmd
+test-fanout
+test-nsutils
 wproc
 snprintf.h
+core

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2749
          http://nagios.svn.sourceforge.net/nagios/?rev=2749&view=rev
Author:   ageric
Date:     2013-04-16 12:47:26 +0000 (Tue, 16 Apr 2013)
Log Message:
-----------
lib/runcmd: Grok escaped shell variables inside double-quoted strings

Previously we'd raise a flag for this (or perhaps that was just
because the latest patch), but with this addendum we handle it
properly once again.

While at it, we add a test-case for it to make sure we avoid
regressions.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/lib/runcmd.c
    nagioscore/trunk/lib/test-runcmd.c

Modified: nagioscore/trunk/lib/runcmd.c
===================================================================
--- nagioscore/trunk/lib/runcmd.c	2013-04-16 12:47:07 UTC (rev 2748)
+++ nagioscore/trunk/lib/runcmd.c	2013-04-16 12:47:26 UTC (rev 2749)
@@ -242,7 +242,7 @@
 			break;
 
 		case '$':
-			if (!(state & STATE_INSQ)) {
+			if (!have_state(STATE_INSQ) && !have_state(STATE_BSLASH)) {
 				if (p[1] == '(')
 					add_ret(RUNCMD_HAS_SUBCOMMAND);
 				else

Modified: nagioscore/trunk/lib/test-runcmd.c
===================================================================
--- nagioscore/trunk/lib/test-runcmd.c	2013-04-16 12:47:07 UTC (rev 2748)
+++ nagioscore/trunk/lib/test-runcmd.c	2013-04-16 12:47:26 UTC (rev 2749)
@@ -63,6 +63,8 @@
 	{ 0, "ls -l /dev/tty\\?" },
 	{ RUNCMD_HAS_SHVAR, "echo $foo" },
 	{ 0, "echo \\$foo" },
+	{ RUNCMD_HAS_PAREN, "\\$(hoopla booyaka" },
+	{ 0, "\\$\\(hoopla booyaka" },
 	{ 0, NULL},
 };
 

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2748
          http://nagios.svn.sourceforge.net/nagios/?rev=2748&view=rev
Author:   ageric
Date:     2013-04-16 12:47:07 +0000 (Tue, 16 Apr 2013)
Log Message:
-----------
lib/runcmd: Handle escaped glyphs in double-quoted strings properly

Previously, an argument such as "\\Key\Lalafoo\\Value" would get
translated to "\KeyLalafoo\Value", which is not how the shell does
it.

With this patch we get the properly escaped string \Key\Lalafoo\Value,
which is the same as /bin/echo produces with the above quoting, and
is what Nagios 3 used to execute when given such a string.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/lib/runcmd.c
    nagioscore/trunk/lib/test-runcmd.c

Modified: nagioscore/trunk/lib/runcmd.c
===================================================================
--- nagioscore/trunk/lib/runcmd.c	2013-04-16 12:46:46 UTC (rev 2747)
+++ nagioscore/trunk/lib/runcmd.c	2013-04-16 12:47:07 UTC (rev 2748)
@@ -114,6 +114,7 @@
 #define STATE_INSQ  (1 << 2)
 #define STATE_INDQ  (1 << 3)
 #define STATE_SPECIAL (1 << 4)
+#define STATE_BSLASH (1 << 5)
 #define in_quotes (state & (STATE_INSQ | STATE_INDQ))
 #define is_state(s) (state == s)
 #define set_state(s) (state = s)
@@ -152,8 +153,28 @@
 			break;
 
 		case '\\':
-			if (!have_state(STATE_INSQ))
-				i++;
+			/* single-quoted strings never interpolate backslashes */
+			if (have_state(STATE_INSQ) || have_state(STATE_BSLASH)) {
+				break;
+			}
+			/*
+			 * double-quoted strings let backslashes escape
+			 * a few, but not all, shell specials
+			 */
+			if (have_state(STATE_INDQ)) {
+				const char next = str[i + 1];
+				switch (next) {
+				case '"': case '\\': case '$': case '`':
+					add_state(STATE_BSLASH);
+					continue;
+				}
+				break;
+			}
+			/*
+			 * unquoted strings remove unescaped backslashes,
+			 * but backslashes escape anything and everything
+			 */
+			i++;
 			break;
 
 		case '\'':
@@ -209,7 +230,7 @@
 			break;
 
 		case '`':
-			if (!(state & STATE_INSQ)) {
+			if (!have_state(STATE_INSQ) && !have_state(STATE_BSLASH)) {
 				add_ret(RUNCMD_HAS_SUBCOMMAND);
 			}
 			break;
@@ -240,6 +261,9 @@
 			break;
 		}
 
+		/* here, we're limited to escaped backslashes, so remove STATE_BSLASH */
+		del_state(STATE_BSLASH);
+
 		if (is_state(STATE_NONE)) {
 			set_state(STATE_INARG);
 			out_argv[arg++] = &argz[a];

Modified: nagioscore/trunk/lib/test-runcmd.c
===================================================================
--- nagioscore/trunk/lib/test-runcmd.c	2013-04-16 12:46:46 UTC (rev 2747)
+++ nagioscore/trunk/lib/test-runcmd.c	2013-04-16 12:47:07 UTC (rev 2748)
@@ -11,14 +11,15 @@
 };
 
 struct cases cases[] = {
+	{"test0\\", "test0"},
 	{"te\\st1", "test1"},
 	{"te\\\\st2", "te\\st2"},
 	{"te\\\\\\st3", "te\\st3"},
 	{"te\\\\\\\\st4", "te\\\\st4"},
 
-	{"\"te\\st5\"", "test5"},
+	{"\"te\\st5\"", "te\\st5"},
 	{"\"te\\\\st6\"", "te\\st6"},
-	{"\"te\\\\\\st7\"", "te\\st7"},
+	{"\"te\\\\\\st7\"", "te\\\\st7"},
 	{"\"te\\\\\\\\st8\"", "te\\\\st8"},
 
 	{"'te\\st9'", "te\\st9"},
@@ -28,6 +29,7 @@
 
 	{"\\'te\\\\st13", "'te\\st13"},
 	{"'test14\"'", "test14\""},
+	{"\"\\\\test\"", "\\test"},
 	{NULL},
 };
 
@@ -72,6 +74,14 @@
 } parse_case[] = {
 	{ 0, "foo bar nisse", 3, { "foo", "bar", "nisse", NULL }},
 	{ 0, "foo\\ bar nisse", 2, { "foo bar", "nisse", NULL }},
+	{ 0, "\"\\\\foo\"", 1, { "\\foo", NULL }},
+	{ 0, "\"\\1bs in dq\"", 1, { "\\1bs in dq", NULL }},
+	{ 0, "\"\\\\2bs in dq\"", 1, { "\\2bs in dq", NULL }},
+	{ 0, "\"\\\\\\3bs in dq\"", 1, { "\\\\3bs in dq", NULL }},
+	{ 0, "\"\\\\\\\\4bs in dq\"", 1, { "\\\\4bs in dq", NULL }},
+	{ 0, "\\ \t \\\t  \\ ", 3, { " ", "\t", " ", NULL }},
+	{ 0, "\\$foo walla wonga", 3, { "$foo", "walla", "wonga", NULL }},
+	{ 0, "\"\\$bar is\" very wide open", 4, { "$bar is", "very", "wide", "open", NULL }},
 	{ 0, NULL, 0, { NULL, NULL, NULL }},
 };
 

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2747
          http://nagios.svn.sourceforge.net/nagios/?rev=2747&view=rev
Author:   ageric
Date:     2013-04-16 12:46:46 +0000 (Tue, 16 Apr 2013)
Log Message:
-----------
lib/test-runcmd: Enumerate the anomaly testcases

It's silly not to, since searching for the right C representation
of a shell-escaped string is just retardedly annoying.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/lib/test-runcmd.c

Modified: nagioscore/trunk/lib/test-runcmd.c
===================================================================
--- nagioscore/trunk/lib/test-runcmd.c	2013-04-12 16:44:03 UTC (rev 2746)
+++ nagioscore/trunk/lib/test-runcmd.c	2013-04-16 12:46:46 UTC (rev 2747)
@@ -43,14 +43,14 @@
 	{ 0, "foo \\& bar" },
 	{ RUNCMD_HAS_JOBCONTROL, "lala foo ; bar" },
 	{ 0, "lala 'foo; bar'" },
-	{ RUNCMD_HAS_SUBCOMMAND, "foo \"`extcmd`\"" },
-	{ 0, "foo \"\\`extcmd\\`\"" },
-	{ RUNCMD_HAS_SUBCOMMAND, "foo `extcmd`" },
-	{ 0, "foo \\`extcmd\\`" },
-	{ RUNCMD_HAS_SUBCOMMAND, "foo \"$(extcmd)\"" },
-	{ 0, "foo \\$\\(extcmd\\)" },
-	{ RUNCMD_HAS_SUBCOMMAND | RUNCMD_HAS_PAREN, "foo $(extcmd" },
-	{ 0, "foo \\$\\(extcmd\\)" },
+	{ RUNCMD_HAS_SUBCOMMAND, "foo \"`extcmd1`\"" },
+	{ 0, "foo \"\\`extcmd1\\`\"" },
+	{ RUNCMD_HAS_SUBCOMMAND, "foo `extcmd2`" },
+	{ 0, "foo \\`extcmd2\\`" },
+	{ RUNCMD_HAS_SUBCOMMAND, "foo \"$(extcmd3)\"" },
+	{ 0, "foo \\$\\(extcmd3\\)" },
+	{ RUNCMD_HAS_SUBCOMMAND | RUNCMD_HAS_PAREN, "foo $(extcmd4" },
+	{ 0, "foo \\$\\(extcmd4\\)" },
 	{ RUNCMD_HAS_UBDQ, "foo\" bar" },
 	{ 0, "foo\\\" bar" },
 	{ RUNCMD_HAS_UBSQ, "foo' bar" },

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2746
          http://nagios.svn.sourceforge.net/nagios/?rev=2746&view=rev
Author:   ageric
Date:     2013-04-12 16:44:03 +0000 (Fri, 12 Apr 2013)
Log Message:
-----------
specfile updates by Daniel Wittenberg

Patched-by: Daniel Wittenberg <dwi...@gm...>
Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/nagios.spec

Modified: nagioscore/trunk/nagios.spec
===================================================================
--- nagioscore/trunk/nagios.spec	2013-04-09 18:31:06 UTC (rev 2745)
+++ nagioscore/trunk/nagios.spec	2013-04-12 16:44:03 UTC (rev 2746)
@@ -22,11 +22,13 @@
 Packager: Daniel Wittenberg <dwi...@gm...>
 Vendor: Nagios Enterprises (http://www.nagios.org)
 Source0: http://dl.sf.net/nagios/nagios-%{version}.tar.gz
+Source1: %{_sourcedir}/Nagios-beta.png
 BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
 BuildRequires: gd-devel > 1.8
 BuildRequires: zlib-devel
 BuildRequires: libpng-devel
 BuildRequires: libjpeg-devel
+BuildRequires: doxygen
 
 Obsoletes: nagios-www <= %{version}
 Requires: httpd,php
@@ -95,6 +97,9 @@
 find . -type f -name Makefile -exec /usr/bin/perl -p -i -e "s/-mtune=generic/-march=nocona/g" Makefile {} \; -print
 %{__make} %{?_smp_mflags} all
 
+### Build our documentaiton
+%{__make} dox
+
 ### Apparently contrib does not obey configure !
 %{__make} %{?_smp_mflags} -C contrib
 
@@ -108,6 +113,8 @@
 
 %{__install} -d -m 0755 %{buildroot}%{_includedir}/nagios/
 %{__install} -p -m 0644 include/*.h %{buildroot}%{_includedir}/nagios/
+%{__mkdir} -p -m 0755 %{buildroot}/%{_includedir}/nagios/lib
+%{__install} -m 0644 lib/*.h %{buildroot}/%{_includedir}/nagios/lib
 
 %{__install} -Dp -m 0644 sample-config/httpd.conf %{buildroot}%{_sysconfdir}/httpd/conf.d/nagios.conf
 
@@ -124,7 +131,12 @@
 
 ### Install logos
 %{__mkdir_p} %{buildroot}%{_datadir}/nagios/images/logos
+%{__cp} %{SOURCE1} %{buildroot}%{_datadir}/nagios/images/
 
+### Install documentation
+%{__mkdir_p} %{buildroot}%{_datadir}/nagios/documentation
+%{__cp} -a Documentation/html/* %{buildroot}%{_datadir}/nagios/documentation
+
 # Put the new RC script in place
 %{__install} -m 0755 daemon-init %{buildroot}/%{_initrddir}/nagios
 %{__install} -d -m 0755 %{buildroot}/%{_sysconfdir}/sysconfig/
@@ -135,6 +147,9 @@
     DESTDIR="%{buildroot}" \
     INSTALL_OPTS=""
 
+### Install libnagios
+%{__install} -m 0644 lib/libnagios.a %{buildroot}%{_libdir}/libnagios.a
+
 %{__install} -d -m 0755 %{buildroot}%{_libdir}/nagios/plugins/eventhandlers/
 %{__cp} -afpv contrib/eventhandlers/* %{buildroot}%{_libdir}/nagios/plugins/eventhandlers/
 %{__mv} contrib/README contrib/README.contrib
@@ -193,14 +208,15 @@
 %attr(0755,root,root) %{_bindir}/nagios
 %attr(0755,root,root) %{_bindir}/nagiostats
 %attr(0644,root,root) %{_libdir}/nagios/plugins/
-%attr(0644,root,root) %{_datadir}/nagios/
+%attr(0755,root,root) %{_datadir}/nagios/
 %attr(0755,nagios,nagios) %dir %{_sysconfdir}/nagios/
 %attr(0644,nagios,nagios) %config(noreplace) %{_sysconfdir}/nagios/*.cfg
-%attr(0644,nagios,nagios) %config(noreplace) %{_sysconfdir}/nagios/objects
+%attr(0755,nagios,nagios) %{_sysconfdir}/nagios/objects/
 %attr(0755,nagios,nagios) %dir %{_localstatedir}/nagios/
-%attr(0644,nagios,nagios) %{_localstatedir}/nagios/
-%attr(0644,root,root) %{logdir}/
+%attr(0755,nagios,nagios) %{_localstatedir}/nagios/
+%attr(0755,nagios,nagios) %{logdir}/
 %attr(0755,nagios,apache) %{_localstatedir}/nagios/rw/
+%attr(0644,root,root) %{_libdir}/libnagios.a
 
 %files devel
 %attr(0755,root,root) %{_includedir}/nagios/
@@ -211,6 +227,6 @@
 %attr(0755,root,root) %{_libdir}/nagios/plugins/eventhandlers/
 
 %changelog
-* Fri Sep 14 2012 Daniel Wittenberg <dwi...@gm...> 3.99.95-1
-- Major update for version 4.0
+* Fri Mar 15 2013 Daniel Wittenberg <dwi...@gm...> 3.99.96-1
+- Major updates for version 4.0
 - New spec file, new RC script, new sysconfig

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2745
          http://nagios.svn.sourceforge.net/nagios/?rev=2745&view=rev
Author:   ageric
Date:     2013-04-09 18:31:06 +0000 (Tue, 09 Apr 2013)
Log Message:
-----------
core: Avoid lock file removal race condition

The race condition happens when we receive a SIGHUP, break out
of the event execution loop, and receive any sort of signal
setting the 'sigshutdown' flag AFTER checking for the sigshutdown
state but BEFORE the do-while loop terminates. I.e, during
execution of cleanup() or close_debug_log() towards the end of
the loop.

It's a pretty narrow window, but the result is a segfault on
invalid memory access when we attempt to unlink() the lockfile.

Original-patch-by: Anton L?\195?\182fgren <alo...@op...>
Signed-off-by: Anton L?\195?\182fgren <alo...@op...>
Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/base/nagios.c
    nagioscore/trunk/base/utils.c

Modified: nagioscore/trunk/base/nagios.c
===================================================================
--- nagioscore/trunk/base/nagios.c	2013-04-09 18:30:38 UTC (rev 2744)
+++ nagioscore/trunk/base/nagios.c	2013-04-09 18:31:06 UTC (rev 2745)
@@ -827,10 +827,6 @@
 				iobroker_destroy(nagios_iobs, IOBROKER_CLOSE_SOCKETS);
 				nagios_iobs = NULL;
 
-				/* make sure lock file has been removed - it may not have been if we received a shutdown command */
-				if(daemon_mode == TRUE)
-					unlink(lock_file);
-
 				/* log a shutdown message */
 				logit(NSLOG_PROCESS_INFO, TRUE, "Successfully shutdown... (PID=%d)\n", (int)getpid());
 				}
@@ -844,7 +840,11 @@
 			}
 		while(sigrestart == TRUE && sigshutdown == FALSE);
 
+		if(daemon_mode == TRUE)
+			unlink(lock_file);
+
 		/* free misc memory */
+		my_free(lock_file);
 		my_free(config_file);
 		}
 

Modified: nagioscore/trunk/base/utils.c
===================================================================
--- nagioscore/trunk/base/utils.c	2013-04-09 18:30:38 UTC (rev 2744)
+++ nagioscore/trunk/base/utils.c	2013-04-09 18:31:06 UTC (rev 2745)
@@ -3155,7 +3155,6 @@
 	my_free(temp_path);
 	my_free(check_result_path);
 	my_free(command_file);
-	my_free(lock_file);
 	my_free(log_archive_path);
 
 	return;
@@ -3189,6 +3188,8 @@
 	temp_path = (char *)strdup(DEFAULT_TEMP_PATH);
 	check_result_path = (char *)strdup(DEFAULT_CHECK_RESULT_PATH);
 	command_file = (char *)strdup(DEFAULT_COMMAND_FILE);
+	if (lock_file) /* this is kept across restarts */
+		free(lock_file);
 	lock_file = (char *)strdup(DEFAULT_LOCK_FILE);
 	log_archive_path = (char *)strdup(DEFAULT_LOG_ARCHIVE_PATH);
 	debug_file = (char *)strdup(DEFAULT_DEBUG_FILE);

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2744
          http://nagios.svn.sourceforge.net/nagios/?rev=2744&view=rev
Author:   ageric
Date:     2013-04-09 18:30:38 +0000 (Tue, 09 Apr 2013)
Log Message:
-----------
lib/tests: Janitor patch

Fix a bunch of compiler warnings.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/lib/test-bitmap.c
    nagioscore/trunk/lib/test-dkhash.c
    nagioscore/trunk/lib/test-fanout.c
    nagioscore/trunk/lib/test-iobroker.c
    nagioscore/trunk/lib/test-iocache.c
    nagioscore/trunk/lib/test-kvvec.c
    nagioscore/trunk/lib/test-runcmd.c
    nagioscore/trunk/lib/test-squeue.c

Modified: nagioscore/trunk/lib/test-bitmap.c
===================================================================
--- nagioscore/trunk/lib/test-bitmap.c	2013-04-09 18:29:56 UTC (rev 2743)
+++ nagioscore/trunk/lib/test-bitmap.c	2013-04-09 18:30:38 UTC (rev 2744)
@@ -6,7 +6,7 @@
 int main(int argc, char **argv)
 {
 	bitmap *a = NULL, *b, *r_union, *r_diff, *r_symdiff, *r_intersect;
-	int i;
+	unsigned int i;
 	int sa[] = {    2, 3, 4, 1783, 1784, 1785 };
 	int sb[] = { 1, 2, 3,          1784, 1785, 1786, 1790, 1791, 1792 };
 

Modified: nagioscore/trunk/lib/test-dkhash.c
===================================================================
--- nagioscore/trunk/lib/test-dkhash.c	2013-04-09 18:29:56 UTC (rev 2743)
+++ nagioscore/trunk/lib/test-dkhash.c	2013-04-09 18:30:38 UTC (rev 2744)
@@ -87,7 +87,8 @@
 int main(int argc, char **argv)
 {
 	dkhash_table *tx, *t;
-	int x, ret, r2;
+	unsigned int x;
+	int ret, r2;
 	struct test_data s;
 	char *p1, *p2;
 	char *strs[10];
@@ -180,7 +181,7 @@
 	for(x = 0; x < 10; x++) {
 		dkhash_insert(t, strs[x], NULL, strs[x]);
 	}
-	for(x = 9; x >= 0; x--) {
+	for(x = 9; x; x--) {
 		p1 = strs[x];
 		p2 = dkhash_remove(t, p1, NULL);
 		test(p1 == p2, "remove should return a value");

Modified: nagioscore/trunk/lib/test-fanout.c
===================================================================
--- nagioscore/trunk/lib/test-fanout.c	2013-04-09 18:29:56 UTC (rev 2743)
+++ nagioscore/trunk/lib/test-fanout.c	2013-04-09 18:30:38 UTC (rev 2744)
@@ -1,3 +1,5 @@
+#define _GNU_SOURCE 1
+#include <stdio.h>
 #include "fanout.c"
 #include "t-utils.h"
 
@@ -15,7 +17,7 @@
 
 static void run_tests(int ntests, int fo_size)
 {
-	struct tcase *test;
+	struct tcase *tc;
 	unsigned long last_ptr, *ptr;
 	int i, added = 0, removed = 0;
 	fanout_table *fo;
@@ -23,10 +25,10 @@
 	last_ptr = ntests;
 
 	fo = fanout_create(fo_size);
-	test = calloc(ntests, sizeof(*test));
+	tc = calloc(ntests, sizeof(*tc));
 	for (i = 0; i < ntests; i++) {
-		test[i].value = i;
-		if (!fanout_add(fo, test[i].key, &test[i].value))
+		tc[i].value = i;
+		if (!fanout_add(fo, tc[i].key, &tc[i].value))
 			added++;
 	}
 	ok_int(added, ntests, "Adding stuff must work");
@@ -42,14 +44,14 @@
 
 	fo = fanout_create(fo_size);
 	for (i = 0; i < ntests; i++) {
-		test[i].value = i;
-		if (!fanout_add(fo, test[i].key, &test[i].value))
+		tc[i].value = i;
+		if (!fanout_add(fo, tc[i].key, &tc[i].value))
 			added++;
 	}
 	fanout_destroy(fo, destructor);
 	ok_int(destroyed, ntests, "Expected ntest entries in destructor");
 	destroyed = 0;
-	free(test);
+	free(tc);
 }
 
 struct test_data {
@@ -86,10 +88,10 @@
 	ok_int(fanout_get(fot, 123887987) == NULL, 1,
 		"get on empty table must yield NULL");
 	for (k = 0; k < 16385; k++) {
-		struct test_data *td = calloc(1, sizeof(*td));
-		td->key = k;
-		asprintf(&td->name, "%lu", k);
-		fanout_add(fot, k, td);
+		struct test_data *tdata = calloc(1, sizeof(*td));
+		tdata->key = k;
+		asprintf(&tdata->name, "%lu", k);
+		fanout_add(fot, k, tdata);
 	}
 	td = fanout_get(fot, k - 1);
 	ok_int(td != NULL, 1, "get must get what add inserts");

Modified: nagioscore/trunk/lib/test-iobroker.c
===================================================================
--- nagioscore/trunk/lib/test-iobroker.c	2013-04-09 18:29:56 UTC (rev 2743)
+++ nagioscore/trunk/lib/test-iobroker.c	2013-04-09 18:30:38 UTC (rev 2744)
@@ -60,13 +60,13 @@
 
 		buf[len] = 0;
 
-		test(len == strlen(msg[i]), "len match for message %d", i);
+		test(len == (int)strlen(msg[i]), "len match for message %d", i);
 		test(!memcmp(buf, msg[i], len), "data match for message %d", i);
 	}
 
 	i++;
 
-	if (i < 0 || i >= ARRAY_SIZE(msg)) {
+	if (i < 0 || i >= (int)ARRAY_SIZE(msg)) {
 		fprintf(stderr, "i = %d in connected_handler(). What's up with that?\n", i);
 		return 0;
 	}

Modified: nagioscore/trunk/lib/test-iocache.c
===================================================================
--- nagioscore/trunk/lib/test-iocache.c	2013-04-09 18:29:56 UTC (rev 2743)
+++ nagioscore/trunk/lib/test-iocache.c	2013-04-09 18:30:38 UTC (rev 2744)
@@ -61,7 +61,7 @@
 
 int main(int argc, char **argv)
 {
-	int i;
+	unsigned int i;
 	struct strcode sc[] = {
 		ADDSTR("\n"),
 		ADDSTR("\0\0"),

Modified: nagioscore/trunk/lib/test-kvvec.c
===================================================================
--- nagioscore/trunk/lib/test-kvvec.c	2013-04-09 18:29:56 UTC (rev 2743)
+++ nagioscore/trunk/lib/test-kvvec.c	2013-04-09 18:30:38 UTC (rev 2744)
@@ -143,7 +143,7 @@
 			struct key_value *kv = &k.kv[i];
 			test(kv->key_len == kv->value_len, "%d.%d; key_len=%d; value_len=%d (%s = %s)",
 				 j, i, kv->key_len, kv->value_len, kv->key, kv->value);
-			test(kv->value_len == strlen(kv->value),
+			test(kv->value_len == (int)strlen(kv->value),
 				 "%d.%d; kv->value_len(%d) == strlen(%s)(%d)",
 				 j, i, kv->value_len, kv->value, (int)strlen(kv->value));
 		}

Modified: nagioscore/trunk/lib/test-runcmd.c
===================================================================
--- nagioscore/trunk/lib/test-runcmd.c	2013-04-09 18:29:56 UTC (rev 2743)
+++ nagioscore/trunk/lib/test-runcmd.c	2013-04-09 18:30:38 UTC (rev 2744)
@@ -1,3 +1,4 @@
+#define _GNU_SOURCE
 #include "runcmd.c"
 #include "t-utils.h"
 #include <stdio.h>
@@ -106,8 +107,8 @@
 		for (i = 0; anomaly[i].cmd; i++) {
 			int out_argc;
 			char *out_argv[256];
-			int ret = runcmd_cmd2strv(anomaly[i].cmd, &out_argc, out_argv);
-			ok_int(ret, anomaly[i].ret, anomaly[i].cmd);
+			int result = runcmd_cmd2strv(anomaly[i].cmd, &out_argc, out_argv);
+			ok_int(result, anomaly[i].ret, anomaly[i].cmd);
 		}
 	}
 	r2 = t_end();
@@ -119,9 +120,9 @@
 		for (i = 0; parse_case[i].cmd; i++) {
 			int x, out_argc;
 			char *out_argv[256];
-			int ret = runcmd_cmd2strv(parse_case[i].cmd, &out_argc, out_argv);
+			int result = runcmd_cmd2strv(parse_case[i].cmd, &out_argc, out_argv);
 			out_argv[out_argc] = NULL;
-			ok_int(ret, 0, parse_case[i].cmd);
+			ok_int(result, 0, parse_case[i].cmd);
 			ok_int(out_argc, parse_case[i].argc_exp, parse_case[i].cmd);
 			for (x = 0; x < parse_case[x].argc_exp && out_argv[x]; x++) {
 				ok_str(parse_case[i].argv_exp[x], out_argv[x], "argv comparison test");

Modified: nagioscore/trunk/lib/test-squeue.c
===================================================================
--- nagioscore/trunk/lib/test-squeue.c	2013-04-09 18:29:56 UTC (rev 2743)
+++ nagioscore/trunk/lib/test-squeue.c	2013-04-09 18:30:38 UTC (rev 2744)
@@ -43,7 +43,7 @@
 	squeue_event *evt;
 } sq_test_event;
 
-static unsigned long sq_high = 0;
+static time_t sq_high = 0;
 static int sq_walker(squeue_event *evt, void *arg)
 {
 	static int walks = 0;

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2743
          http://nagios.svn.sourceforge.net/nagios/?rev=2743&view=rev
Author:   ageric
Date:     2013-04-09 18:29:56 +0000 (Tue, 09 Apr 2013)
Log Message:
-----------
lib/squeue: Protect against insane input

If a user would want to schedule things beyond the unix millenium,
which ends 2038-01-19 03:14:07 UTC (apparently a tuesday, according
to reliable sources), they would previously end up in one of two
unpleasant codepaths. If they were on 32-bit systems, the time would
silently wrap around into negative time, which would be interpreted
as "now" or pretty much anytime inside the 68 years that encompass
the unix millenium, depending on how much they wrapped around. Most
of the time it would be "now" though, since we discard times in the
past inside the squeue library.

If, however, they were on 64-bit systems, the situation could get a
bit more serious, since we would then try to cram 16 bytes of data
into 8 bytes. We would succeed (after a fashion), but we would lose
the top 32 bits of the seconds value for the purpose of priority
queue comparisons, but we would retain them for the sake of the
caller and use it when someone requested it from us (via
squeue_event_runtime()). Since the erroneous event went to the
head of the queue but was in reality scheduled to happen X thousand
years into the future, Nagios wouldn't run it, but would instead
go and sulk in a corner. Not very nice.

This patch fixes it, and as a nice bonus protects against broken
gettimeofday() implementations (are there any?), by masking off
the bottom 21 bits for the microsecond value and the 43 topmost
bits for the tv_sec value we actually store for the timeval
struct.

That means the Nagios millenium will end with the unix millenium
for 32 bit systems and sometime in the year 141166 for 64-bit
systems.

We don't support scheduling things beyond those years on the given
architectures. Doing so will cause undefined behaviour, and we
can't protect against it due to type restrictions enforced on us
by the compiler. Users who try will see their events being run
inappropriately early.

We will, however, not *stop* running events now, but merrily
carry on with our other scheduled tasks after we've run the
poorly scheduled event (at the wrong time).

Reported-by: Ton Voon <ton...@op...>
Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/lib/squeue.c

Modified: nagioscore/trunk/lib/squeue.c
===================================================================
--- nagioscore/trunk/lib/squeue.c	2013-04-09 18:29:36 UTC (rev 2742)
+++ nagioscore/trunk/lib/squeue.c	2013-04-09 18:29:56 UTC (rev 2743)
@@ -28,7 +28,16 @@
 	void *data;
 };
 
-
+/*
+ * 21 bits has enough data for systems that can have the usec
+ * field of a struct timeval move into the 1-second range, but
+ * not enough to let them to (far) beyond 2. If the system libs
+ * are too buggy, we really can't save it.
+ * This little twiddling operation lets us use dates beyond
+ * 2038 on 64-bit systems, while retaining the fast priority
+ * comparisons.
+ */
+#define SQ_BITS 21
 static pqueue_pri_t evt_compute_pri(struct timeval *tv)
 {
 	pqueue_pri_t ret;
@@ -39,8 +48,8 @@
 		ret += !!tv->tv_usec;
 	} else {
 		ret = tv->tv_sec;
-		ret <<= 32;
-		ret |= tv->tv_usec;
+		ret <<= SQ_BITS;
+		ret |= tv->tv_usec & ((1 << SQ_BITS) - 1);
 	}
 
 	return ret;
@@ -108,6 +117,15 @@
 	if (tv->tv_sec < time(NULL))
 		tv->tv_sec = time(NULL);
 	evt->when.tv_sec = tv->tv_sec;
+	if (sizeof(evt->when.tv_sec) > 4) {
+		/*
+		 * Only use bottom sizeof(pqueue_pri_t)-SQ_BITS bits on
+		 * 64-bit systems, or we may get entries at the head
+		 * of the queue are actually scheduled to run several
+		 * hundred thousand years from now.
+		 */
+		evt->when.tv_sec &= (1ULL << ((sizeof(pqueue_pri_t) * 8) - SQ_BITS)) - 1;
+	}
 	evt->when.tv_usec = tv->tv_usec;
 	evt->data = data;
 

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2742
          http://nagios.svn.sourceforge.net/nagios/?rev=2742&view=rev
Author:   ageric
Date:     2013-04-09 18:29:36 +0000 (Tue, 09 Apr 2013)
Log Message:
-----------
base/workers: Fortify job-to-worker assignments

Previously, we could end up doing one of two unpleasant things when
assigning worker to a job and we had no general-purpose workers left
alive.

If we had special-purpose workers, we would end up leaking memory
when asked to run a job that wasn't connected to us.

If we didn't have special purpose workers, we would end up crashing
when asked to run any job at all.

Both scenarios required, as stated above, that all general-purpose
workers had died.

The memory leak was reported by Max Sikstr?\195?\182m <msi...@op...>
during code review. Many thanks.

Signed-off-by: Max Sikstr?\195?\182m <msi...@op...>
Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/base/workers.c

Modified: nagioscore/trunk/base/workers.c
===================================================================
--- nagioscore/trunk/base/workers.c	2013-04-09 18:29:12 UTC (rev 2741)
+++ nagioscore/trunk/base/workers.c	2013-04-09 18:29:36 UTC (rev 2742)
@@ -168,15 +168,10 @@
 	if (wp_list != NULL) {
 		log_debug_info(DEBUGL_CHECKS, 1, "Found specialized worker(s) for '%s'", (slash && *slash != '/') ? slash : cmd_name);
 	}
-	else {
-		if (!workers.wps)
-			return NULL;
-		wp_list = &workers;
-	}
-	if (cmd_name) {
+	if (cmd_name)
 		free(cmd_name);
-	}
-	return wp_list;
+
+	return wp_list ? wp_list : &workers;
 }
 
 static struct wproc_worker *get_worker(const char *cmd)
@@ -187,7 +182,7 @@
 		return NULL;
 
 	wp_list = get_wproc_list(cmd);
-	if (!wp_list)
+	if (!wp_list || !wp_list->wps || !wp_list->len)
 		return NULL;
 
 	return wp_list->wps[wp_list->idx++ % wp_list->len];

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2741
          http://nagios.svn.sourceforge.net/nagios/?rev=2741&view=rev
Author:   ageric
Date:     2013-04-09 18:29:12 +0000 (Tue, 09 Apr 2013)
Log Message:
-----------
base/workers: Assign worker process list len defensively

When we remove a worker, we know there are 'j' workers left in
the list, so in case we've managed to insert a worker twice in
the same list (unlikely but possible), and that worker later
disconnects, we would be left with a len mismatch and crash on
a segfault when attempting to access the now-empty worker slot.

Reported-by: Max Sikstr?\195?\182m <msi...@op...>
Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/base/workers.c

Modified: nagioscore/trunk/base/workers.c
===================================================================
--- nagioscore/trunk/base/workers.c	2013-04-08 12:45:53 UTC (rev 2740)
+++ nagioscore/trunk/base/workers.c	2013-04-09 18:29:12 UTC (rev 2741)
@@ -350,13 +350,13 @@
 			continue;
 		wpl->wps[j++] = wpl->wps[i];
 	}
-	wpl->len--;
+	wpl->len = j;
+
 	if (!specialized_workers || wpl->len)
 		return;
 
 	to_remove = wpl;
 	dkhash_walk_data(specialized_workers, remove_specialized);
-
 }
 
 

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2740
          http://nagios.svn.sourceforge.net/nagios/?rev=2740&view=rev
Author:   ageric
Date:     2013-04-08 12:45:53 +0000 (Mon, 08 Apr 2013)
Log Message:
-----------
lib/fanout: Protect against destructors calling fanout_remove()

If a destructor called by fanout_destroy() in turn calls
fanout_remove() to remove the entry currently being destroyed, we
would previously crash out on a double-free() of the entry we
were destroying.

That's bad. This patch prevents that, letting users shoot themselves
in the foot as much as they like. It does make it impossible for a
destructor to locate information in the table though, but since we
can't switch to a readonly state anyway, we simply ignore that.

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/lib/fanout.c

Modified: nagioscore/trunk/lib/fanout.c
===================================================================
--- nagioscore/trunk/lib/fanout.c	2013-04-07 16:55:59 UTC (rev 2739)
+++ nagioscore/trunk/lib/fanout.c	2013-04-08 12:45:53 UTC (rev 2740)
@@ -29,23 +29,28 @@
 void fanout_destroy(fanout_table *t, void (*destructor)(void *))
 {
 	unsigned long i;
-	struct fanout_entry *next;
+	struct fanout_entry **entries, *next;
 
 	if (!t || !t->entries || !t->alloc)
 		return;
 
+	/* protect against destructors calling fanout_remove() */
+	entries = t->entries;
+	t->entries = NULL;
+
 	for (i = 0; i < t->alloc; i++) {
 		struct fanout_entry *entry;
-		for (entry = t->entries[i]; entry; entry = next) {
+
+		for (entry = entries[i]; entry; entry = next) {
+			void *data = entry->data;
 			next = entry->next;
+			free(entry);
 			if (destructor) {
-				destructor(entry->data);
+				destructor(data);
 			}
-			free(entry);
 		}
-		t->entries[i] = NULL;
 	}
-	free(t->entries);
+	free(entries);
 	free(t);
 }
 

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

Revision: 2739
          http://nagios.svn.sourceforge.net/nagios/?rev=2739&view=rev
Author:   ageric
Date:     2013-04-07 16:55:59 +0000 (Sun, 07 Apr 2013)
Log Message:
-----------
Add Anton Lofgren to THANKS file

Signed-off-by: Andreas Ericsson <ae...@op...>

Modified Paths:
--------------
    nagioscore/trunk/THANKS

Modified: nagioscore/trunk/THANKS
===================================================================
--- nagioscore/trunk/THANKS	2013-04-07 15:46:51 UTC (rev 2738)
+++ nagioscore/trunk/THANKS	2013-04-07 16:55:59 UTC (rev 2739)
@@ -159,6 +159,7 @@
 * Christoph Kron
 * Ivan Kuncl
 * Dean Lane
+* Anton Lofgren
 * Ingo Lantschner
 * Gerhard Lausser
 * William Leibzon

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.