Skip to contentSkip to conversation

git-repack --max-pack-size: make close optional in sha1close()

PATCH In review Dana How 2 participants 3 messages Jun 15, 2016

lore
Share & tools
lore

Dana How

author
#0 lore raw

PATCH 3/8 git-repack --max-pack-size: make close optional in sha1close()

sha1close() flushes, writes checksum, and closes. The 2nd can be suppressed; make the last suppressible as well.

Signed-off-by Dana L. How <[email protected]>
---
 csum-file.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
csum-file.c +3 -0
index 7c806ad..993c899 100644
@@ -35,7 +35,10 @@ int sha1close(struct sha1file *f, unsigned char *result, int update)
 	if (offset) {
 		SHA1_Update(&f->ctx, f->buffer, offset);
 		sha1flush(f, offset);
+		f->offset = 0;
 	}
+	if (update < 0)
+		return 0;	/* only want to flush (no checksum write, no close) */
 	SHA1_Final(f->buffer, &f->ctx);
 	if (result)
 		hashcpy(result, f->buffer);
Signature · 1 line
1.5.2.rc0.766.gba60-dirty
#1 lore raw
Quoted 2 lines · Dana How

sha1close() flushes, writes checksum, and closes. The 2nd can be suppressed; make the last suppressible as well.

...

Quoted 5 lines
csum-file.c +0 -0
index 7c806ad..993c899 100644

csum-file.c int sha1close(struct sha1file *f, unsigned char *result, int update) · line 44

Quoted 10 lines
csum-file.c +3 -0
@@ -35,7 +35,10 @@ int sha1close(struct sha1file *f, unsigned char *result, int update)
 	if (offset) {
 		SHA1_Update(&f->ctx, f->buffer, offset);
 		sha1flush(f, offset);
+		f->offset = 0;
 	}
+	if (update < 0)
+		return 0;	/* only want to flush (no checksum write, no close) */
 	SHA1_Final(f->buffer, &f->ctx);
 	if (result)
 		hashcpy(result, f->buffer);

Huh. Nobody currently uses that update parameter; all current in-tree callers (which better be *all* callers since we don't have a true libgit!) seem to always pass a 1 for this argument. This makes the later:

  if (update)
    sha1flush(f, 20);

always true anytime sha1close is called. Maybe we should be redefining that update argument to be 1 means do all work, 0 means return where you return update < 0 above?

Signature · 1 line
Shawn.
author
#2 lore raw
Quoted 18 lines · Shawn O. Pearce
Quoted 2 lines · Dana How

sha1close() flushes, writes checksum, and closes. The 2nd can be suppressed; make the last suppressible as well.

...

csum-file.c int sha1close(struct sha1file *f, unsigned char *result, int update) · line 38

Quoted 8 lines
csum-file.c +3 -0
@@ -35,7 +35,10 @@ int sha1close(struct sha1file *f, unsigned char *result, int update)
+             f->offset = 0;
      }
+     if (update < 0)
+             return 0;       /* only want to flush (no checksum write, no close) */

Huh. Nobody currently uses that update parameter; all current in-tree callers (which better be *all* callers since we don't have a true libgit!) seem to always pass a 1 for this argument. This makes the later:

Quoted 6 lines
  if (update)
    sha1flush(f, 20);

always true anytime sha1close is called. Maybe we should be redefining that update argument to be 1 means do all work, 0 means return where you return update < 0 above?

Considering I had to look at all the sha1close call sites to confirm -1 was a safe new value,
I should have reached your conclusion as well.
But I was really trying to focus on additions, not alterations.

I'll either roll this into my next patchset (also in pack-objects), or include it in any other changes to this patchset if any.

Thanks,

Signature · 1 line
Dana L. How  [email protected]  +1 650 804 5991 cell