Question about unpack-trees.c and tree_entry_list

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Question about unpack-trees.c and tree_entry_list

From: David Kastrup <hidden>
Date: 2016-06-15 22:43:23

The definition of tree_entry_list ist the following

struct tree_entry_list {
	struct tree_entry_list *next;
	unsigned directory : 1;
	unsigned executable : 1;
	unsigned symlink : 1;
	unsigned int mode;
	const char *name;
	const unsigned char *sha1;
};

but it appears to me that the only of the bit fields that is used at
all is "directory" : all other uses revert to "mode" which directory
presumably could do as well.

Is there something I am overlooking?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[PATCH] cleanup unpack-trees.c: shrink struct tree_entry_list

From: René Scharfe <hidden>
Date: 2016-06-15 22:43:23

David Kastrup schrieb:
The definition of tree_entry_list ist the following

struct tree_entry_list {
	struct tree_entry_list *next;
	unsigned directory : 1;
	unsigned executable : 1;
	unsigned symlink : 1;
	unsigned int mode;
	const char *name;
	const unsigned char *sha1;
};

but it appears to me that the only of the bit fields that is used at
all is "directory" : all other uses revert to "mode" which directory
presumably could do as well.

Is there something I am overlooking?
A C compiler can give the definite answer: no.
--- 8< ---
Remove the two write-only fields executable and symlink from struct
tree_entry_list.  Also replace usage of the field directory with
S_ISDIR checks on the mode field, and then remove this now obsolete
field, too.  Noticed by David Kastrup.

Signed-off-by: Rene Scharfe <redacted>
---
This patch slightly reduces RAM usage, but it also shrinks the code
without reducing functionality or readability, which is more
interesting.

 unpack-trees.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 7cc029e..3b32718 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -11,9 +11,6 @@
 
 struct tree_entry_list {
 	struct tree_entry_list *next;
-	unsigned directory : 1;
-	unsigned executable : 1;
-	unsigned symlink : 1;
 	unsigned int mode;
 	const char *name;
 	const unsigned char *sha1;
@@ -38,9 +35,6 @@ static struct tree_entry_list *create_tree_entry_list(struct tree *tree)
 		entry->name = one.path;
 		entry->sha1 = one.sha1;
 		entry->mode = one.mode;
-		entry->directory = S_ISDIR(one.mode) != 0;
-		entry->executable = (one.mode & S_IXUSR) != 0;
-		entry->symlink = S_ISLNK(one.mode) != 0;
 		entry->next = NULL;
 
 		*list_p = entry;
@@ -141,9 +135,9 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,
 #endif
 			if (!first || entcmp(first, firstdir,
 					     posns[i]->name,
-					     posns[i]->directory) > 0) {
+					     S_ISDIR(posns[i]->mode)) > 0) {
 				first = posns[i]->name;
-				firstdir = posns[i]->directory;
+				firstdir = S_ISDIR(posns[i]->mode);
 			}
 		}
 		/* No name means we're done */
@@ -177,7 +171,7 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,
 				continue;
 			}
 
-			if (posns[i]->directory) {
+			if (S_ISDIR(posns[i]->mode)) {
 				struct tree *tree = lookup_tree(posns[i]->sha1);
 				any_dirs = 1;
 				parse_tree(tree);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help