[PATCH] diff: bug fixes and cleanups. -55 bytes

Matheus Izvekov mizvekov at gmail.com
Tue Jan 12 15:52:07 UTC 2010


>From 5fd06c169ff89f88796efada6b1c67c16caf0618 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov <mizvekov at gmail.com>
Date: Tue, 12 Jan 2010 13:39:29 -0200
Subject: [PATCH] diff: bug fixes and cleanups. -55 bytes

This patch depends on my previous patch, which is mostly a rewrite.

Fixes to a ton of memory leaks.
Valgrind is perfectly happy with this applet now.

Fixes a copy and paste bug introduced in the previous commit.

Now -S flag behaves exactly how it does on gnu diff. It does not error
out in case it would make the list empty.

Diffing a file against a directory does not care about trailing slashes,
to match behavior of gnu diff.
---
 editors/diff.c |  171 ++++++++++++++++++++++++++++---------------------------
 1 files changed, 87 insertions(+), 84 deletions(-)

diff --git a/editors/diff.c b/editors/diff.c
index fdb2855..2087aaa 100644
--- a/editors/diff.c
+++ b/editors/diff.c
@@ -124,18 +124,12 @@ enum flags {               /* Commandline flags */
 struct globals {
 	smallint exit_status;
 	int opt_U_context;
-	char *opt_S_start;
-	IF_FEATURE_DIFF_DIR(int dl_count;)
-	IF_FEATURE_DIFF_DIR(char **dl;)
-	const char *label[2];
+	char *label[2];
 	struct stat stb[2];
 };
 #define G (*ptr_to_globals)
 #define exit_status        (G.exit_status       )
 #define opt_U_context      (G.opt_U_context     )
-#define opt_S_start        (G.opt_S_start       )
-#define dl_count           (G.dl_count          )
-#define dl                 (G.dl                )
 #define label              (G.label             )
 #define stb                (G.stb               )
 #define INIT_G() do { \
@@ -484,7 +478,7 @@ static int *create_J(int fd[2], int nlen[2], off_t *ix[2])
 
 	class = (int *)nfile[0];
 	unsort(sfile[0], slen[0], (int *)nfile[0]);
-	nfile[0] = xrealloc(nfile[0], (slen[0] + 2) * sizeof(int));
+	class = xrealloc(class, (slen[0] + 2) * sizeof(int));
 #endif
 	J = xmalloc((nlen[0] + 2) * sizeof(int));
 	/* The elements of J which fall inside the prefix and suffix regions
@@ -744,19 +738,19 @@ static void print_status(int val, char *_path[2])
 }
 
 #if ENABLE_FEATURE_DIFF_DIR
-static void do_diff(char *dir[2], const char *path[2])
+static void do_diff(char *file[2], char *path[2])
 {
 	int flags = 0; /*D_HEADER;*/
 	char *fullpath[2]; /* if -N */
 
 	for (int i = 0; i < 2; i++) {
-		fullpath[i] = path[i] ? concat_path_file(dir[i], path[i]) : NULL;
+		fullpath[i] = path[i] ? concat_path_file(file[i], path[i]) : NULL;
 		if (!fullpath[i] || stat(fullpath[i], &stb[i]) != 0) {
 			flags |= 1 << (D_EMPTY0 + i);
 			memset(&stb[i], 0, sizeof(stb[i]));
 			if (path[!i]) {
 				free(fullpath[i]);
-				fullpath[i] = concat_path_file(dir[i], path[!i]);
+				fullpath[i] = concat_path_file(file[i], path[!i]);
 			}
 		}
 	}
@@ -783,91 +777,92 @@ static void do_diff(char *dir[2], const char *path[2])
 	free(fullpath[1]);
 }
 
+struct dlist {
+	size_t len;
+	int s, e;
+	char **dl;
+};
+
 /* This function adds a filename to dl, the directory listing. */
 static int FAST_FUNC add_to_dirlist(const char *filename,
 		struct stat *sb UNUSED_PARAM,
-		void *userdata,
+		union {
+			void *userdata;
+			struct dlist *l;
+		} __attribute__((transparent_union)) user,
 		int depth UNUSED_PARAM)
 {
-	dl = xrealloc_vector(dl, 5, dl_count);
-	dl[dl_count] = xstrdup(filename + (int)(ptrdiff_t)userdata);
-	dl_count++;
-	return TRUE;
+	user.l->dl = xrealloc_vector(user.l->dl, 5, user.l->e);
+	user.l->dl[user.l->e++] = xstrdup(filename + user.l->len);
+	return true;
 }
 
-/* This returns a sorted directory listing. */
-static char **get_recursive_dirlist(const char *path)
+/* This function adds a filename to dl, the directory listing. */
+static int FAST_FUNC skip_dir(const char *filename UNUSED_PARAM,
+		struct stat *sb UNUSED_PARAM,
+		void *userdata UNUSED_PARAM,
+		int depth)
 {
-	dl_count = 0;
-	dl = xzalloc(sizeof(dl[0]));
-
-	/* We need to trim root directory prefix.
-	 * Using void *userdata to specify its length,
-	 * add_to_dirlist will remove it. */
-	if (option_mask32 & FLAG(r)) {
-		recursive_action(path, ACTION_RECURSE|ACTION_FOLLOWLINKS,
-					add_to_dirlist, /* file_action */
-					NULL, /* dir_action */
-					(void*)(ptrdiff_t)(strlen(path) + 1),
-					0);
-	} else {
-		DIR *dp = warn_opendir(path);
-		
-		for (struct dirent *ep; (ep = readdir(dp)); ) {
-			if (!strcmp(ep->d_name, "..") || LONE_CHAR(ep->d_name, '.'))
-				continue;
-			add_to_dirlist(ep->d_name, NULL, (void*)(int)0, 0);
-		}
-		closedir(dp);
+	if (!(option_mask32 & FLAG(r)) && depth) {
+		add_to_dirlist(filename, sb, userdata, depth);
+		return SKIP;
 	}
-
-	/* Sort dl alphabetically. */
-	qsort_string_vector(dl, dl_count);
-
-	dl[dl_count] = NULL;
-	return dl;
+	return true;
 }
 
-static void diffdir(char *p[2])
+static void diffdir(char *p[2], const char *s_start)
 {
-	char **dirlist[2];
+	struct dlist list[2];
 
 	for (int i = 0; i < 2; i++) {
-		/* Check for trailing slashes. */
-		char *dp = last_char_is(p[i], '/');
-		if (dp != NULL)
-			*dp = '\0';
-		/* Get directory listings for p1 and p2. */
-		dirlist[i] = get_recursive_dirlist(p[i]);
+		list[i].s = list[i].e = 0;
+		list[i].dl = NULL;
+		/* We need to trim root directory prefix.
+		 * Using list.len to specify its length,
+		 * add_to_dirlist will remove it. */
+		list[i].len = strlen(p[i]) + 1;
+		recursive_action(p[i], ACTION_RECURSE | ACTION_FOLLOWLINKS,
+		                 add_to_dirlist, skip_dir, &list[i], 0);
+		/* Sort dl alphabetically.
+		 * GNU diff does this ignoring any number of trailing dots
+		 * We don't, so for us dotted files almost always are
+		 * first on the list
+		 */
+		qsort_string_vector(list[i].dl, list[i].e);
 		/* If -S was set, find the starting point. */
-		if (opt_S_start) {
-			while (*dirlist[i] != NULL && strcmp(*dirlist[i], opt_S_start) < 0)
-				dirlist[i]++;
-			if (*dirlist[i] == NULL)
-				bb_error_msg(bb_msg_invalid_arg, "NULL", "-S");
-		}
+		if (!s_start)
+			continue;
+		for (; list[i].s < list[i].e &&
+		       strcmp(list[i].dl[list[i].s], s_start) < 0; list[i].s++);
 	}
 	/* Now that both dirlist1 and dirlist2 contain sorted directory
 	 * listings, we can start to go through dirlist1. If both listings
 	 * contain the same file, then do a normal diff. Otherwise, behaviour
 	 * is determined by whether the -N flag is set. */
-	while (*dirlist[0] != NULL || *dirlist[1] != NULL) {
-		const char *dp[2] = { *dirlist[0], *dirlist[1] };
-		int pos = dp[0] == NULL ? 1 : (dp[1] == NULL ? -1 : strcmp(dp[0], dp[1]));
-		bool i = pos > 0;
-		if (pos)
-			dp[!i] = NULL;
+	while (1) {
+		char *dp[2];
+		int pos;
+		bool k;
+
+		for (int j = 0; j < 2; j++)
+			dp[j] = list[j].s < list[j].e ? list[j].dl[list[j].s] : NULL;
+		if (!dp[0] && !dp[1])
+			break;
+		pos = !dp[0] ? 1 : (!dp[1] ? -1 : strcmp(dp[0], dp[1]));
+		k = pos > 0;
 		if (pos && !(option_mask32 & FLAG(N)))
-			printf("Only in %s: %s\n", p[i], dp[i]);
+			printf("Only in %s: %s\n", p[k], dp[k]);
 		else
 			do_diff(p, dp);
-		if (pos)
-			dirlist[i]++;
-		else {
-			dirlist[0]++;
-			dirlist[1]++;
+		free(dp[k]);
+		list[k].s++;
+		if (!pos) {
+			free(dp[!k]);
+			list[!k].s++;
 		}
 	}
+	free(list[0].dl);
+	free(list[1].dl);
 }
 #endif
 
@@ -875,8 +870,9 @@ int diff_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int diff_main(int argc UNUSED_PARAM, char **argv)
 {
 	int gotstdin = 0;
-	char *file[2];
+	char *file[2], *s_start = NULL;
 	llist_t *L_arg = NULL;
+	bool dir, dirfile;
 
 	INIT_G();
 
@@ -884,13 +880,15 @@ int diff_main(int argc UNUSED_PARAM, char **argv)
 	opt_complementary = "=2:L::U+";
 	getopt32(argv, "abdiL:NqrsS:tTU:wu"
 			"p" /* ignored (for compatibility) */,
-			&L_arg, &opt_S_start, &opt_U_context);
+			&L_arg, &s_start, &opt_U_context);
 	argv += optind;
 	while (L_arg) {
 		if (label[0] && label[1])
 			bb_show_usage();
-		if (label[0]) /* then label[1] is NULL */
+		if (label[0]) { /* then label[1] is NULL */
+			free(label[1]);
 			label[1] = label[0];
+		}
 		label[0] = llist_pop(&L_arg);
 	}
 	xfunc_error_retval = 2;
@@ -909,24 +907,29 @@ int diff_main(int argc UNUSED_PARAM, char **argv)
 
 	if (S_ISDIR(stb[0].st_mode) && S_ISDIR(stb[1].st_mode)) {
 #if ENABLE_FEATURE_DIFF_DIR
-		diffdir(file);
+		diffdir(file, s_start);
 		return exit_status;
 #else
 		bb_error_msg_and_die("no support for directory comparison");
 #endif
 	}
 
-	for (int i = 0; i < 2; i++)
-		if (S_ISDIR(stb[i].st_mode)) { /* "diff dir file" */
-			/* NB: "diff dir      dir2/dir3/file" must become
-			 *     "diff dir/file dir2/dir3/file" */
-			const char *slash = strrchr(file[!i], '/');
-			file[i] = concat_path_file(file[i], slash ? slash + 1 : file[!i]);
-			xstat(file[i], &stb[i]);
-		}
-
+	dirfile = S_ISDIR(stb[0].st_mode) || S_ISDIR(stb[1].st_mode);
+	dir = S_ISDIR(stb[1].st_mode);
+	if (dirfile) {
+		const char *slash = strrchr(file[!dir], '/');
+		file[dir] = concat_path_file(file[dir], slash ? slash + 1 : file[!dir]);
+		xstat(file[dir], &stb[dir]);
+	}
 	/* diffreg can get non-regular files here,
 	 * they are not both DIRestories */
 	print_status((gotstdin > 1 ? STATUS_SAME : diffreg(file, 0)), file);
+
+	if (dirfile)
+		free(file[dir]);
+
+	free(label[0]);
+	free(label[1]);
+
 	return exit_status;
 }
-- 
1.6.6



More information about the busybox mailing list