[PATCH] ls: clean up memory leak

Shaun Jackman sjackman at gmail.com
Fri Apr 28 14:10:39 UTC 2006


As per recent discussion. Please apply.

Cheers,
Shaun

2006-04-28  Shaun Jackman  <sjackman at gmail.com>

	* coreutils/ls.c: Clean up memory leak.

--- coreutils/ls.c	bb33fd3b5686e379b3bf13c7d395ce88d9f3ebc3
+++ coreutils/ls.c	8d0559ef2e0de18dd205c78939665419cdb41c32
@@ -38,7 +38,6 @@

 #include <sys/types.h>
 #include <sys/stat.h>
-#include <stdio.h>
 #include <unistd.h>
 #include <dirent.h>
 #include <errno.h>
@@ -176,16 +175,17 @@
 struct dnode {			/* the basic node */
 	char *name;		/* the dir entry name */
 	char *fullname;		/* the dir entry name */
-	int   allocated;
 	struct stat dstat;	/* the file stat info */
 #ifdef CONFIG_SELINUX
 	security_context_t sid;
 #endif
-	struct dnode *next;	/* point at the next node */
+	/* point at the next dnode used in list_dir only
+	   and not null if fullname have memory allocated */
+	void *allocated;
 };
 typedef struct dnode dnode_t;

-static struct dnode **list_dir(const char *);
+static struct dnode **list_dir(const char *, int *pnfiles);
 static struct dnode **dnalloc(int);
 static int list_single(struct dnode *);

@@ -245,6 +245,7 @@
 	cur->fullname = fullname;
 	cur->name = name;
 	cur->dstat = dstat;
+	cur->allocated = NULL;
 #ifdef CONFIG_SELINUX
 	cur->sid = sid;
 #endif
@@ -311,20 +312,6 @@
 	return (dirs);
 }

-static int countfiles(struct dnode **dnp)
-{
-	int nfiles;
-	struct dnode *cur;
-
-	if (dnp == NULL)
-		return (0);
-	nfiles = 0;
-	for (cur = dnp[0]; cur->next != NULL; cur = cur->next)
-		nfiles++;
-	nfiles++;
-	return (nfiles);
-}
-
 /* get memory to hold an array of pointers */
 static struct dnode **dnalloc(int num)
 {
@@ -338,20 +325,16 @@
 }

 #ifdef CONFIG_FEATURE_LS_RECURSIVE
-static void dfree(struct dnode **dnp)
+static void dfree(struct dnode **dnp, int nfiles)
 {
-	struct dnode *cur, *next;
+	struct dnode *cur;
+	int n;

-	if (dnp == NULL)
-		return;
-
-	cur = dnp[0];
-	while (cur != NULL) {
+	for(n = 0; n < nfiles; n++) {
+		cur = dnp[n];
 		if(cur->allocated)
 			free(cur->fullname);	/* free the filename */
-		next = cur->next;
 		free(cur);		/* free the dnode */
-		cur = next;
 	}
 	free(dnp);			/* free the array holding the dnode pointers */
 }
@@ -474,7 +457,11 @@
 	} else {
 		/* find the longest file name-  use that as the column width */
 		for (i = 0; i < nfiles; i++) {
+#ifdef CONFIG_FEATURE_UTF8_SUPPORT
+			int len = mbchar_len(dn[i]->name) +
+#else
 			int len = strlen(dn[i]->name) +
+#endif
 #ifdef CONFIG_SELINUX
 			((all_fmt & LIST_CONTEXT) ? 33 : 0) +
 #endif
@@ -540,8 +527,7 @@
 			first = 0;
 			printf("%s:\n", dn[i]->fullname);
 		}
-		subdnp = list_dir(dn[i]->fullname);
-		nfiles = countfiles(subdnp);
+		subdnp = list_dir(dn[i]->fullname, &nfiles);
 		if (nfiles > 0) {
 			/* list all files at this level */
 #ifdef CONFIG_FEATURE_LS_SORTFILES
@@ -561,14 +547,14 @@
 					free(dnd);	/* free the array of dnode pointers to the dirs */
 				}
 			}
-			dfree(subdnp);	/* free the dnodes and the fullname mem */
+			dfree(subdnp, nfiles);  /* free the dnodes and the fullname mem */
 #endif
 		}
 	}
 }

 /*----------------------------------------------------------------------*/
-static struct dnode **list_dir(const char *path)
+static struct dnode **list_dir(const char *path, int *pnfiles)
 {
 	struct dnode *dn, *cur, **dnp;
 	struct dirent *entry;
@@ -602,24 +588,25 @@
 		cur = my_stat(fullname, strrchr(fullname, '/') + 1);
 		if (!cur)
 			continue;
-		cur->allocated = 1;
-		cur->next = dn;
+		cur->allocated = dn;
 		dn = cur;
 		nfiles++;
 	}
 	closedir(dir);

+	*pnfiles = nfiles;
+	if (nfiles == 0)
+		return NULL;
+
 	/* now that we know how many files there are
 	   ** allocate memory for an array to hold dnode pointers
 	 */
-	if (dn == NULL)
-		return (NULL);
 	dnp = dnalloc(nfiles);
 	for (i = 0, cur = dn; i < nfiles; i++) {
 		dnp[i] = cur;	/* save pointer to node in array */
-		cur = cur->next;
+		cur = cur->allocated;
 	}
-
+	dnp[i-1]->allocated = dn;
 	return (dnp);
 }

@@ -947,7 +934,6 @@
 	struct dnode **dnd;
 	struct dnode **dnf;
 	struct dnode **dnp;
-	struct dnode *dn;
 	struct dnode *cur;
 	long opt;
 	int nfiles = 0;
@@ -1115,26 +1101,14 @@
 		all_fmt |= DISP_DIRNAME;	/* 2 or more items? label directories */

 	/* stuff the command line file names into an dnode array */
-	dn = NULL;
+	dnp = dnalloc(ac);
 	for (oi = 0; oi < ac; oi++) {
 		cur = my_stat(av[oi], av[oi]);
 		if (!cur)
 			continue;
-		cur->allocated = 0;
-		cur->next = dn;
-		dn = cur;
-		nfiles++;
+		dnp[nfiles++] = cur;
 	}

-	/* now that we know how many files there are
-	   ** allocate memory for an array to hold dnode pointers
-	 */
-	dnp = dnalloc(nfiles);
-	for (i = 0, cur = dn; i < nfiles; i++) {
-		dnp[i] = cur;	/* save pointer to node in array */
-		cur = cur->next;
-	}
-
 	if (all_fmt & DISP_NOLIST) {
 #ifdef CONFIG_FEATURE_LS_SORTFILES
 		shellsort(dnp, nfiles);
@@ -1151,13 +1125,19 @@
 			shellsort(dnf, dnfiles);
 #endif
 			showfiles(dnf, dnfiles);
+			if (ENABLE_FEATURE_CLEAN_UP)
+				free(dnf);
 		}
 		if (dndirs > 0) {
 #ifdef CONFIG_FEATURE_LS_SORTFILES
 			shellsort(dnd, dndirs);
 #endif
 			showdirs(dnd, dndirs, dnfiles == 0);
+			if (ENABLE_FEATURE_CLEAN_UP)
+				free(dnd);
 		}
 	}
+	if (ENABLE_FEATURE_CLEAN_UP)
+		dfree(dnp, nfiles);
 	return (status);
 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ls-cleanup.diff
Type: text/x-patch
Size: 5199 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20060428/42d98046/attachment.bin 


More information about the busybox mailing list