From: Andrés J. Díaz <ajdiaz@connectical.com>
Date: Thu, 17 Sep 2009 11:15:02 +0000 (+0200)
Subject: processes plugin: Remove unnecessary call of realloc(3).
X-Git-Tag: collectd-4.7.4~8
X-Git-Url: https://git.tokkee.org/?a=commitdiff_plain;h=c327b3ae6b19713a56631e81886988121d3eef93;p=collectd.git

processes plugin: Remove unnecessary call of realloc(3).

Hi Florian (et al)

> you're right, the (re-)allocation of the memory can probably be avoided
> if the function is turned into one with the following prototype:
> -- 8< --
>  static int *ps_read_tasks (int pid,
>     unsigned long *ret_num_proc,
>     unsigned long *ret_num_lwp);
> -- >8 --

Mmm, why not something like: "static int ps_read_task(pid)"?
This returns the number of task for pid passed as argument.
(AFAIK the function only return the number ot threads), why
we need the ret_num_proc and the ret_num_lwp parameters?

My proposal is attached (code is always cleaner than explanations :P)

Regards,
  Andres

Signed-off-by: Florian Forster <octo@huhu.verplant.org>
---

diff --git a/src/processes.c b/src/processes.c
index 261accbc..8337f99a 100644
--- a/src/processes.c
+++ b/src/processes.c
@@ -617,65 +617,31 @@ static void ps_submit_proc_list (procstat_t *ps)
 
 /* ------- additional functions for KERNEL_LINUX/HAVE_THREAD_INFO ------- */
 #if KERNEL_LINUX
-static int *ps_read_tasks (int pid)
+static int ps_read_tasks (int pid)
 {
-	int *list = NULL;
-	int  list_size = 1; /* size of allocated space, in elements */
-	int  list_len = 0;  /* number of currently used elements */
-
 	char           dirname[64];
 	DIR           *dh;
 	struct dirent *ent;
+	int count = 0;
 
 	ssnprintf (dirname, sizeof (dirname), "/proc/%i/task", pid);
 
 	if ((dh = opendir (dirname)) == NULL)
 	{
 		DEBUG ("Failed to open directory `%s'", dirname);
-		return (NULL);
+		return (-1);
 	}
 
 	while ((ent = readdir (dh)) != NULL)
 	{
 		if (!isdigit (ent->d_name[0]))
 			continue;
-
-		if ((list_len + 1) >= list_size)
-		{
-			int *new_ptr;
-			int  new_size = 2 * list_size;
-			/* Comes in sizes: 2, 4, 8, 16, ... */
-
-			new_ptr = (int *) realloc (list, (size_t) (sizeof (int) * new_size));
-			if (new_ptr == NULL)
-			{
-				if (list != NULL)
-					free (list);
-				ERROR ("processes plugin: "
-						"Failed to allocate more memory.");
-				return (NULL);
-			}
-
-			list = new_ptr;
-			list_size = new_size;
-
-			memset (list + list_len, 0, sizeof (int) * (list_size - list_len));
-		}
-
-		list[list_len] = atoi (ent->d_name);
-		if (list[list_len] != 0)
-			list_len++;
+		else
+			count++;
 	}
-
 	closedir (dh);
 
-	if (list_len == 0)
-		return (NULL);
-
-	assert (list_len < list_size);
-	assert (list[list_len] == 0);
-
-	return (list);
+	return (count?count:1);
 } /* int *ps_read_tasks */
 
 int ps_read_process (int pid, procstat_t *ps, char *state)
@@ -686,7 +652,6 @@ int ps_read_process (int pid, procstat_t *ps, char *state)
 	char *fields[64];
 	char  fields_len;
 
-	int  *tasks;
 	int   i;
 
 	int   ppid;
@@ -736,21 +701,11 @@ int ps_read_process (int pid, procstat_t *ps, char *state)
 		ps->num_lwp  = 0;
 		ps->num_proc = 0;
 	}
-	else if ((tasks = ps_read_tasks (pid)) == NULL)
-	{
-		/* Kernel 2.4 or so */
-		ps->num_lwp  = 1;
-		ps->num_proc = 1;
-	}
 	else
 	{
-		ps->num_lwp  = 0;
+		if ( (ps->num_lwp = ps_read_tasks (pid)) == -1 )
+			return (-1);
 		ps->num_proc = 1;
-		for (i = 0; tasks[i] != 0; i++)
-			ps->num_lwp++;
-
-		free (tasks);
-		tasks = NULL;
 	}
 
 	/* Leave the rest at zero if this is only a zombi */