[PATCH] Produce a nicer output in case of sha1_object_info failures in ls-tree -l

[Alex Riesen <raa.lkml@gmail.com>]

An error message is already printed by sha1_object_info itself, and
the failed entries are additionally marked in the listing.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Junio C Hamano, Thu, Mar 19, 2009 23:13:10 +0100:
> Alex Riesen <raa.lkml@gmail.com> writes:
> 
> > Junio C Hamano, Thu, Mar 19, 2009 22:55:56 +0100:
> >> Alex Riesen <raa.lkml@gmail.com> writes:
> >> > @@ -91,6 +90,7 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,
> >> >  	if (!(ls_options & LS_NAME_ONLY)) {
> >> >  		if (ls_options & LS_SHOW_SIZE) {
> >> >  			if (!strcmp(type, blob_type)) {
> >> > +				unsigned long size = 0;
> >> >  				sha1_object_info(sha1, &size);
> >> >  				printf("%06o %s %s %7lu\t", mode, type,
> >> >  				       abbrev ? find_unique_abbrev(sha1, abbrev)
> >> 
> >> Hmm, shouldn't you be checking the return value from sha1_object_info()
> >> and skipping the printf() altogether instead?
> >
> > But then I cannot know the name of the failed tree entry.
> 
> Why?
> 
> 	if (sha1_object_info() == OBJ_BAD)
> 		die("object recorded at tree entry %s is bad", pathname);
> 	printf ...

Tried. Makes exactly this code much uglier, and the pathname is
printed nicely quoted after the outer if() is closed. And I don't like
the idea of dying here: it'll take longer to collect all the needed
entry names for later recovery (that's how it came to the change,
AFAIR).

How about this patch instead? I chose "BAD" for the marker, as any
automatic processing trying blindly to convert it into a number will
get a 0, which seems safe to me.

 builtin-ls-tree.c |   22 ++++++++++++----------
 1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
index fca4631..22008df 100644
--- a/builtin-ls-tree.c
+++ b/builtin-ls-tree.c
@@ -60,7 +60,6 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,
 {
 	int retval = 0;
 	const char *type = blob_type;
-	unsigned long size;
 
 	if (S_ISGITLINK(mode)) {
 		/*
@@ -90,17 +89,20 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,
 
 	if (!(ls_options & LS_NAME_ONLY)) {
 		if (ls_options & LS_SHOW_SIZE) {
+			char size_text[24];
 			if (!strcmp(type, blob_type)) {
-				sha1_object_info(sha1, &size);
-				printf("%06o %s %s %7lu\t", mode, type,
-				       abbrev ? find_unique_abbrev(sha1, abbrev)
-				              : sha1_to_hex(sha1),
-				       size);
+				unsigned long size;
+				if (sha1_object_info(sha1, &size) == OBJ_BAD)
+					strcpy(size_text, "BAD");
+				else
+					snprintf(size_text, sizeof(size_text),
+						 "%lu", size);
 			} else
-				printf("%06o %s %s %7c\t", mode, type,
-				       abbrev ? find_unique_abbrev(sha1, abbrev)
-				              : sha1_to_hex(sha1),
-				       '-');
+				strcpy(size_text, "-");
+			printf("%06o %s %s %7s\t", mode, type,
+			       abbrev ? find_unique_abbrev(sha1, abbrev)
+				      : sha1_to_hex(sha1),
+			       size_text);
 		} else
 			printf("%06o %s %s\t", mode, type,
 			       abbrev ? find_unique_abbrev(sha1, abbrev)
-- 
1.6.2.1.237.g7206c6