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

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

[Dana How]

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

Signed-off-by: Dana How <how@deathvalley.cswitch.com>
---
 csum-file.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/csum-file.c b/csum-file.c
index b7174c6..e1ff769 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -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);
-- 
1.5.1.89.g8abf0

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

[Dana How]

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

Signed-off-by: Dana L. How <danahow@gmail.com>
---
 csum-file.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/csum-file.c b/csum-file.c
index 7c806ad..993c899 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -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);
-- 
1.5.2.rc0.766.gba60-dirty

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

[Shawn O. Pearce]

Dana How <danahow@gmail.com> wrote:
> sha1close() flushes, writes checksum, and closes.
> The 2nd can be suppressed; make the last suppressible as well.
...
> diff --git a/csum-file.c b/csum-file.c
> index 7c806ad..993c899 100644
> --- a/csum-file.c
> +++ b/csum-file.c
> @@ -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?

-- 
Shawn.

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

[Dana How]

On 4/30/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> Dana How <danahow@gmail.com> wrote:
> > sha1close() flushes, writes checksum, and closes.
> > The 2nd can be suppressed; make the last suppressible as well.
> ...
> > diff --git a/csum-file.c b/csum-file.c
> > --- a/csum-file.c
> > +++ b/csum-file.c
> > @@ -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:
>
>   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,
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell