[icinga-checkins] icinga.org: icinga-core/mfriedrich/core: core: fix allocate memory once for *GROUPMEMBERS macros (Stephane Lapie) #1076

git at icinga.org git at icinga.org
Wed Jan 5 15:57:39 CET 2011


Module: icinga-core
Branch: mfriedrich/core
Commit: 54978065368514b14b1cbec8fcfbc91d05dd3275
URL:    https://git.icinga.org/?p=icinga-core.git;a=commit;h=54978065368514b14b1cbec8fcfbc91d05dd3275

Author: Michael Friedrich <michael.friedrich at univie.ac.at>
Date:   Wed Jan  5 15:55:54 2011 +0100

core: fix allocate memory once for *GROUPMEMBERS macros (Stephane Lapie) #1076

[--SNIP--]
Previously we used to iterate over all available members once and
issue a realloc for every member. Such memory thrashing provides
very poor performance compared to just counting the length we need
and allocate it properly once and then writing to the buffer thus
allocated.

This patch corrects that for host and servicegroupmembers.

Signed-off-by: Stephane Lapie <stephane.lapie at darkbsd.org>
Signed-off-by: Andreas Ericsson <ae at op5.se>

Author: Stephane Lapie <stephane.lapie at darkbsd.org>
[--SNIP--]

fixes #1076

---

 Changelog       |    1 +
 THANKS          |    1 +
 common/macros.c |   68 ++++++++++++++++++++++++++++++++++++++++++-------------
 3 files changed, 54 insertions(+), 16 deletions(-)

diff --git a/Changelog b/Changelog
index 3d1dfc9..c242fb4 100644
--- a/Changelog
+++ b/Changelog
@@ -46,6 +46,7 @@ FIXES
 * core: debug logging is now properly serialized, using soft-locking with a timeout of 150 milliseconds to avoid multiple threads competing for the privilege to write debug info (Andreas Ericsson) #1035
 * core: fix parsing of long plugin output for async host checks (Jochen Bern) #1046
 * core: log error reason when failing to open the status file (Andreas Ericsson) #1078
+* core: fix allocate memory once for *GROUPMEMBERS macros (Stephane Lapie) #1076
 
 * classic ui: change servicestatus letter color to default black, not grey #946
 * classic ui: fix waste of cpu in status summary (TomTom) #933
diff --git a/THANKS b/THANKS
index fb5713f..ce95a0f 100644
--- a/THANKS
+++ b/THANKS
@@ -169,6 +169,7 @@ in various ways.  If we missed your name, let us know.
 * Ivan Kuncl
 * Dean Lane
 * Ingo Lantschner
+* Stephane Lapie
 * Gerhard Lausser
 * William Leibzon
 * Pedro Leite
diff --git a/common/macros.c b/common/macros.c
index 70f549b..71553f9 100644
--- a/common/macros.c
+++ b/common/macros.c
@@ -1836,6 +1836,8 @@ int grab_standard_host_macro(icinga_macros *mac, int macro_type, host *temp_host
 int grab_standard_hostgroup_macro(icinga_macros *mac, int macro_type, hostgroup *temp_hostgroup, char **output){
 	hostsmember *temp_hostsmember=NULL;
 	char *temp_buffer=NULL;
+	unsigned int temp_len=0;
+	unsigned int init_len=0;
 
 	if(temp_hostgroup==NULL || output==NULL)
 		return ERROR;
@@ -1850,15 +1852,33 @@ int grab_standard_hostgroup_macro(icinga_macros *mac, int macro_type, hostgroup
 			*output=(char *)strdup(temp_hostgroup->alias);
 		break;
 	case MACRO_HOSTGROUPMEMBERS:
-		/* get the group members */
+		/* make the calculations for total string length */
 		for(temp_hostsmember=temp_hostgroup->members;temp_hostsmember!=NULL;temp_hostsmember=temp_hostsmember->next){
 			if(temp_hostsmember->host_name==NULL)
 				continue;
-			if(*output==NULL)
-				*output=(char *)strdup(temp_hostsmember->host_name);
-			else if((*output=(char *)realloc(*output,strlen(*output)+strlen(temp_hostsmember->host_name)+2))){
-				strcat(*output,",");
-				strcat(*output,temp_hostsmember->host_name);
+			if (temp_len==0){
+				temp_len+=strlen(temp_hostsmember->host_name)+1;
+			} else {
+				temp_len+=strlen(temp_hostsmember->host_name)+2;
+				}
+			}
+		/* allocate or reallocate the memory buffer */
+		if (*output==NULL) {
+			*output=(char *)malloc(temp_len);
+		} else {
+			init_len = strlen(*output);
+			temp_len += init_len;
+			*output=(char *)realloc(*output,temp_len);
+		}
+		/* now fill in the string with the member names */
+		for(temp_hostsmember=temp_hostgroup->members;temp_hostsmember!=NULL;temp_hostsmember=temp_hostsmember->next){
+			if(temp_hostsmember->host_name==NULL)
+				continue;
+			temp_buffer = *output + init_len;
+			if (init_len == 0) { /* If our buffer didn't contain anything, we just need to write "%s,%s" */
+				init_len += sprintf(temp_buffer, "%s", temp_hostsmember->host_name);
+			} else {
+				init_len += sprintf(temp_buffer, ",%s", temp_hostsmember->host_name);
 				}
 			}
 		break;
@@ -2141,6 +2161,8 @@ int grab_standard_service_macro(icinga_macros *mac, int macro_type, service *tem
 int grab_standard_servicegroup_macro(icinga_macros *mac, int macro_type, servicegroup *temp_servicegroup, char **output){
 	servicesmember *temp_servicesmember=NULL;
 	char *temp_buffer=NULL;
+	unsigned int temp_len=0;
+	unsigned int init_len=0;
 
 	if(temp_servicegroup==NULL || output==NULL)
 		return ERROR;
@@ -2155,20 +2177,34 @@ int grab_standard_servicegroup_macro(icinga_macros *mac, int macro_type, service
 			*output=(char *)strdup(temp_servicegroup->alias);
 		break;
 	case MACRO_SERVICEGROUPMEMBERS:
-		/* get the group members */
+		/* make the calculations for total string length */
 		for(temp_servicesmember=temp_servicegroup->members;temp_servicesmember!=NULL;temp_servicesmember=temp_servicesmember->next){
 			if(temp_servicesmember->host_name==NULL || temp_servicesmember->service_description==NULL)
 				continue;
-			if(*output==NULL){
-				if((*output=(char *)malloc(strlen(temp_servicesmember->host_name)+strlen(temp_servicesmember->service_description)+2))){
-					sprintf(*output,"%s,%s",temp_servicesmember->host_name,temp_servicesmember->service_description);
-					}
+			if (temp_len == 0) {
+				temp_len+=strlen(temp_servicesmember->host_name)+strlen(temp_servicesmember->service_description)+2;
+			} else {
+				temp_len+=strlen(temp_servicesmember->host_name)+strlen(temp_servicesmember->service_description)+3;
 				}
-			else if((*output=(char *)realloc(*output,strlen(*output)+strlen(temp_servicesmember->host_name)+strlen(temp_servicesmember->service_description)+3))){
-				strcat(*output,",");
-				strcat(*output,temp_servicesmember->host_name);
-				strcat(*output,",");
-				strcat(*output,temp_servicesmember->service_description);
+			}
+		/* allocate or reallocate the memory buffer */
+		if (*output==NULL) {
+			*output=(char *)malloc(temp_len);
+			}
+		else {
+			init_len = strlen(*output);
+			temp_len += init_len;
+			*output=(char *)realloc(*output,temp_len);
+			}
+		/* now fill in the string with the group members */
+		for(temp_servicesmember=temp_servicegroup->members;temp_servicesmember!=NULL;temp_servicesmember=temp_servicesmember->next){
+			if(temp_servicesmember->host_name==NULL || temp_servicesmember->service_description==NULL)
+				continue;
+			temp_buffer = *output + init_len;
+			if (init_len == 0) { /* If our buffer didn't contain anything, we just need to write "%s,%s" */
+				init_len += sprintf(temp_buffer, "%s,%s",temp_servicesmember->host_name,temp_servicesmember->service_description);
+			} else { /* Now we need to write ",%s,%s" */
+				init_len += sprintf(temp_buffer, ",%s,%s",temp_servicesmember->host_name,temp_servicesmember->service_description);
 				}
 			}
 		break;





More information about the icinga-checkins mailing list