[PATCH] ls: clean up

Shaun Jackman sjackman at gmail.com
Tue Apr 25 17:00:39 UTC 2006


On 4/25/06, Vladimir N. Oleynik <dzo at simtreas.ru> wrote:
> > I agree, sort transposes the elements without deleting them. You will
> > notice though, that countfiles() is never called on a sorted array.
> > The number of files is counted before sorting. Now that I look at it
> > though, countfiles is entirely superfluous. list_dir should take an
> > `int *nfiles' argument instead.
>
> Yes. I too have thought of it. ;-) Look a patch for attach.
> It will really give a prize in size.

Yes! I like this patch. Please apply it. However, you missed three
memory leaks. You might find dmalloc helps to ferret out these subtle
(or not-so-subtle) bugs down. Please apply this patch as well.

On i386, the total savings are 48 bytes of .text.

Cheers,
Shaun

--- coreutils/ls.c-	2006-04-25 10:38:29.000000000 -0600
+++ coreutils/ls.c	2006-04-25 10:55:00.000000000 -0600
@@ -38,10 +38,10 @@

 #include <sys/types.h>
 #include <sys/stat.h>
-#include <stdio.h>
 #include <unistd.h>
 #include <dirent.h>
 #include <errno.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
@@ -565,7 +565,7 @@
 	if (path == NULL)
 		return (NULL);

-	dn = NULL;
+	dn = (void*)(intptr_t)1;
  	nfiles = 0;
  	dir = bb_opendir(path);
  	if (dir == NULL) {
@@ -594,12 +594,12 @@
  		nfiles++;
 	}
  	closedir(dir);
+	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 */
@@ -1124,12 +1124,16 @@
  			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)
-------------- next part --------------
--- coreutils/ls.c-	2006-04-25 10:38:29.000000000 -0600
+++ coreutils/ls.c	2006-04-25 10:55:00.000000000 -0600
@@ -38,10 +38,10 @@
 
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <stdio.h>
 #include <unistd.h>
 #include <dirent.h>
 #include <errno.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
@@ -565,7 +565,7 @@
 	if (path == NULL)
 		return (NULL);
 
-	dn = NULL;
+	dn = (void*)(intptr_t)1;
 	nfiles = 0;
 	dir = bb_opendir(path);
 	if (dir == NULL) {
@@ -594,12 +594,12 @@
 		nfiles++;
 	}
 	closedir(dir);
+	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 */
@@ -1124,12 +1124,16 @@
 			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)



More information about the busybox mailing list