threads / patch / 6479

patchsha1_file.c: Avoid multiple calls to find_pack_entry().

Subject: [PATCH] sha1_file.c: Avoid multiple calls to find_pack_entry().

## tl;dr

7 messages between Jan 22, 2007 and Jan 23, 2007. Diffs are folded; open one to read it.

replies: 6people: 6as markdown or json

Peter Eriksen· Jan 22, 2007, 20:29 UTC · lore

We used to call find_pack_entry() twice from read_sha1_file() in order to avoid printing an error message, when the object did not exist. This is fixed by moving the call to error() to the only place it really could be called.

Signed-off-by: Peter Eriksen <s022018@student.dtu.dk>
---
 sha1_file.c |   19 +++++++++----------
 1 files changed, 9 insertions(+), 10 deletions(-)
Show changes to sha1_file.c +9 −10
diff --git a/sha1_file.c b/sha1_file.c
index 3025440..43ff402 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1469,21 +1469,20 @@ static void *read_packed_sha1(const unsigned char *sha1, char *type, unsigned lo
 {
 	struct pack_entry e;
 
-	if (!find_pack_entry(sha1, &e, NULL)) {
-		error("cannot read sha1_file for %s", sha1_to_hex(sha1));
+	if (!find_pack_entry(sha1, &e, NULL))
 		return NULL;
-	}
-	return unpack_entry(e.p, e.offset, type, size);
+	else
+		return unpack_entry(e.p, e.offset, type, size);
 }
 
 void * read_sha1_file(const unsigned char *sha1, char *type, unsigned long *size)
 {
 	unsigned long mapsize;
 	void *map, *buf;
-	struct pack_entry e;
 
-	if (find_pack_entry(sha1, &e, NULL))
-		return read_packed_sha1(sha1, type, size);
+	buf = read_packed_sha1(sha1, type, size);
+	if (buf)
+		return buf;
 	map = map_sha1_file(sha1, &mapsize);
 	if (map) {
 		buf = unpack_sha1_file(map, mapsize, type, size);
@@ -1491,9 +1490,7 @@ void * read_sha1_file(const unsigned char *sha1, char *type, unsigned long *size
 		return buf;
 	}
 	reprepare_packed_git();
-	if (find_pack_entry(sha1, &e, NULL))
-		return read_packed_sha1(sha1, type, size);
-	return NULL;
+	return read_packed_sha1(sha1, type, size);
 }
 
 void *read_object_with_reference(const unsigned char *sha1,
@@ -1781,6 +1778,8 @@ static void *repack_object(const unsigned char *sha1, unsigned long *objsize)
 
 	/* need to unpack and recompress it by itself */
 	unpacked = read_packed_sha1(sha1, type, &len);
+	if (!unpacked)
+		error("cannot read sha1_file for %s", sha1_to_hex(sha1));
 
 	hdrlen = sprintf(hdr, "%s %lu", type, len) + 1;
 
-- 
1.5.0.rc1.gdf1b-dirty
Simon 'corecode' Schubert· Jan 22, 2007, 20:56 UTC · re: Peter Eriksen · lore

Re: [PATCH] sha1_file.c: Avoid multiple calls to find_pack_entry().

Peter Eriksen wrote:
Show 6 quoted lines
> We used to call find_pack_entry() twice from read_sha1_file() in order
> to avoid printing an error message, when the object did not exist.  This
> is fixed by moving the call to error() to the only place it really
> could be called.
> 
> Signed-off-by: Peter Eriksen <s022018@student.dtu.dk>
I noticed this originally, Peter was so kind to come up with a patch.  Reviewed and found +1, so:
Signed-off-by: Simon 'corecode' Schubert <corecode@fs.ei.tum.de>
cheers
  simon
-- 
Serve - BSD     +++  RENT this banner advert  +++    ASCII Ribbon   /"\
Work - Mac      +++  space for low €€€ NOW!1  +++      Campaign     \ /
Party Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \
Dude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \
Shawn O. Pearce· Jan 22, 2007, 21:07 UTC · re: Simon 'corecode' Schubert · lore

Re: [PATCH] sha1_file.c: Avoid multiple calls to find_pack_entry().

Simon 'corecode' Schubert <corecode@fs.ei.tum.de> wrote:
Show 12 quoted lines
> Peter Eriksen wrote:
> >We used to call find_pack_entry() twice from read_sha1_file() in order
> >to avoid printing an error message, when the object did not exist.  This
> >is fixed by moving the call to error() to the only place it really
> >could be called.
> >
> >Signed-off-by: Peter Eriksen <s022018@student.dtu.dk>
> 
> I noticed this originally, Peter was so kind to come up with a patch.  
> Reviewed and found +1, so:
> 
> Signed-off-by: Simon 'corecode' Schubert <corecode@fs.ei.tum.de>
Wow.  That's ugly.  Thanks for finding and patching it.
-- 
Shawn.
Linus Torvalds· Jan 23, 2007, 00:25 UTC · re: Simon 'corecode' Schubert · lore

Re: [PATCH] sha1_file.c: Avoid multiple calls to find_pack_entry().

On Mon, 22 Jan 2007, Simon 'corecode' Schubert wrote:
Show 6 quoted lines
> 
> -- 
> Serve - BSD     +++  RENT this banner advert  +++    ASCII Ribbon   /"\
> Work - Mac      +++  space for low  NOW!1  +++      Campaign     \ /
> Party Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \
> Dude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \

Whee. It's been a long time since I saw ascii barfics and posted them to alt.fan.warlord, and I doubt the newsgroup even exists any more, but I have to commend you for getting a tab-damaged ascii barfic even without using any TAB characters!

I assume there are three '$$$' characters missing in your banner advertizing space. Maybe an Euro character? The joys of tab-damage just expands in the modern world of new character sets..

For extra points, please add ascii runes and/or big ascii swords to your signature, to make it truly old-time warlord material.

		Linus "nostalgic for afw" Torvalds
Jakub Narebski· Jan 23, 2007, 00:39 UTC · re: Linus Torvalds · lore

Re: [PATCH] sha1_file.c: Avoid multiple calls to find_pack_entry().

Linus Torvalds wrote:
> I assume there are three '$$$' characters missing in your banner 
> advertizing space. Maybe an Euro character? The joys of tab-damage just 
> expands in the modern world of new character sets..

I see three euro characters there. I don't know why unknown character didn't got replaced by ? or something like that....

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Morten Welinder· Jan 23, 2007, 02:10 UTC · re: Linus Torvalds · lore

Re: [PATCH] sha1_file.c: Avoid multiple calls to find_pack_entry().

>                 Linus "nostalgic for afw" Torvalds
>From the "nostalgic" department we also have this.  I have no idea if it

will survive today's mailing lists. In any case, a fixed-width font is your friend. (And I sadly note that linux.cs.helsinki.fi is nowadays refusing finger connections.)

Morten (who lost the original attribution -- sorry. I certainly didn't make it myself.)

                                   _,-/"---,
            ;"""""""""";         _/;; ""  <@`---v
          ; :::::  ::  "\      _/ ;;  "    _.../
         ;"     ;;  ;;;  \___/::    ;;,'""""
        ;"          ;;;;.  ;;  ;;;  ::/
       ,/ / ;;  ;;;______;;;  ;;; ::,/
       /;;V_;;   ;;;       \       /
       | :/ / ,/            \_ "")/
       | | / /"""=            \;;\""=
       ; ;{::""""""=            \"""=
    ;"""";
    \/"""
         THAT's a weasel, folks!
              Linux 2.0
Linus Torvalds· Jan 23, 2007, 03:17 UTC · re: Morten Welinder · lore

Re: [PATCH] sha1_file.c: Avoid multiple calls to find_pack_entry().

On Mon, 22 Jan 2007, Morten Welinder wrote:
>
> (And I sadly note that linux.cs.helsinki.fi is nowadays refusing finger 
> connections.)

I think the ascii barfics went away even before I left University due to a dead harddisk, and me being too lazy to re-create my changing fingerd and all the cruddy ascii art.

I think Helsinki University ended up having a simple finger service for a while to return the versioning thing (just because it was in too many FAQ's to drop entirely), but I don't think the great artwork was ever recreated..

(In all honesty, I don't think it ever really had more than a couple of pictures - the weasel, the sleeping cat, and some random other ones. "Great art" it was not..)

		Linus

← back to recent threads