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

git at icinga.org git at icinga.org
Thu Jan 13 15:57:30 CET 2011


Module: icinga-core
Branch: r1.2
Commit: 341ee472c8883ca072f6b0e834016d8ad62402f4
URL:    https://git.icinga.org/?p=icinga-core.git;a=commit;h=341ee472c8883ca072f6b0e834016d8ad62402f4

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 de4ef13..52f8e1a 100644
--- a/Changelog
+++ b/Changelog
@@ -25,6 +25,7 @@ FIXES
 * core: fix NOTIFICATIONTYPE MACRO never became CUSTOM (Alexey Dvoryanchikov) #950
 * core: fix parsing of long plugin output for async host checks (Jochen Bern) #1046
 * core: fix possible validation error with empty hostgroups/servicegroups (Sven-Göran Bergh) #1040
+* 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 9117962..a38552d 100644
--- a/THANKS
+++ b/THANKS
@@ -168,6 +168,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 e7463b4..e58e144 100644
--- a/common/macros.c
+++ b/common/macros.c
@@ -1824,6 +1824,8 @@ int grab_standard_host_macro(int macro_type, host *temp_host, char **output, int
 int grab_standard_hostgroup_macro(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;
@@ -1838,15 +1840,33 @@ int grab_standard_hostgroup_macro(int macro_type, hostgroup *temp_hostgroup, cha
 			*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;
@@ -2129,6 +2149,8 @@ int grab_standard_service_macro(int macro_type, service *temp_service, char **ou
 int grab_standard_servicegroup_macro(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;
@@ -2143,20 +2165,34 @@ int grab_standard_servicegroup_macro(int macro_type, servicegroup *temp_serviceg
 			*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