Re: [PATCH v3 5/5] archive-tar: use internal gzip by default
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jun 14, 2022, 16:29 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2206141802310.353@tvgsbejvaqbjf.bet>
- In-Reply-To
- <28f6ec2a-1d94-b29a-4bfd-6a9e74c8edbf@web.de>
Hi René,
On Tue, 14 Jun 2022, René Scharfe wrote:
Show 46 quoted lines
> Am 14.06.22 um 13:27 schrieb Johannes Schindelin:
>
> > The solution is to move the heap variable back into a scope that matches
> > the lifetime of the compression:
> >
> > -- snip --
> > diff --git a/archive-tar.c b/archive-tar.c
> > index 60669eb7b9c..3d77e0f7509 100644
> > --- a/archive-tar.c
> > +++ b/archive-tar.c
> > @@ -460,17 +460,12 @@ static void tgz_write_block(const void *data)
> >
> > static const char internal_gzip_command[] = "git archive gzip";
> >
> > -static void tgz_set_os(git_zstream *strm, int os)
> > -{
> > -#if ZLIB_VERNUM >= 0x1221
> > - struct gz_header_s gzhead = { .os = os };
> > - deflateSetHeader(&strm->z, &gzhead);
> > -#endif
> > -}
> > -
> > static int write_tar_filter_archive(const struct archiver *ar,
> > struct archiver_args *args)
> > {
> > +#if ZLIB_VERNUM >= 0x1221
> > + struct gz_header_s gzhead = { .os = 3 }; /* Unix, for reproducibility */
> > +#endif
> > struct strbuf cmd = STRBUF_INIT;
> > struct child_process filter = CHILD_PROCESS_INIT;
> > int r;
> > @@ -481,7 +476,10 @@ static int write_tar_filter_archive(const struct archiver *ar,
> > if (!strcmp(ar->filter_command, internal_gzip_command)) {
> > write_block = tgz_write_block;
> > git_deflate_init_gzip(&gzstream, args->compression_level);
> > - tgz_set_os(&gzstream, 3); /* Unix, for reproducibility */
> > +#if ZLIB_VERNUM >= 0x1221
> > + if (deflateSetHeader(&gzstream.z, &gzhead) != Z_OK)
> > + BUG("deflateSetHeader() called too late");
> > +#endif
> > gzstream.next_out = outbuf;
> > gzstream.avail_out = sizeof(outbuf);
> >
> > -- snap --
>
> Good find, thank you! A shorter solution would be to make gzhead static.I should have said that I had considered this, but decided against it because it would introduce yet another issue: it would render the code needlessly un-multi-threadable. And that can be avoided _really_ easily.
Ciao, Dscho