threads / patch / 7915

patch, 8 partsgit-repack --max-pack-size: make close optional in sha1close()

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

## tl;dr

3 messages between Apr 30, 2007 and May 1, 2007. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Dana How· Apr 30, 2007, 23:20 UTC · lore

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(-)

Show changes to csum-file.c +3 −0

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

Shawn O. Pearce· May 1, 2007, 05:01 UTC · re: Dana How · lore

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

Dana How <danahow@gmail.com> wrote:

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

...

Show 15 quoted lines

> 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.

Dana How· May 1, 2007, 05:24 UTC · re: Shawn O. Pearce · lore

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

On 4/30/07, Shawn O. Pearce <spearce@spearce.org> wrote:

Show 24 quoted lines

> 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