On Tue, Dec 21 2021, Ævar Arnfjörð Bjarmason wrote:
> On Tue, Dec 21 2021, Han Xin wrote:
>> + /* Then the data itself.. */
>> + do {
>> + unsigned char *in0 = stream.next_in;
>> + if (!stream.avail_in) {
>> + const void *in = in_stream->read(in_stream, &stream.avail_in);
>> + stream.next_in = (void *)in;
>> + in0 = (unsigned char *)in;
>> + /* All data has been read. */
>> + if (len + hdrlen == stream.total_in + stream.avail_in)
>> + flush = Z_FINISH;
>> + }
>> + ret = git_deflate(&stream, flush);
>> + the_hash_algo->update_fn(&c, in0, stream.next_in - in0);
>> + if (write_buffer(fd, compressed, stream.next_out - compressed) < 0)
>> + die(_("unable to write loose object file"));
>> + stream.next_out = compressed;
>> + stream.avail_out = sizeof(compressed);
>> + } while (ret == Z_OK || ret == Z_BUF_ERROR);
>> +
>> + if (ret != Z_STREAM_END)
>> + die(_("unable to deflate new object streamingly (%d)"), ret);
>> + ret = git_deflate_end_gently(&stream);
>> + if (ret != Z_OK)
>> + die(_("deflateEnd on object streamingly failed (%d)"), ret);
>
> nit: let's say "unable to stream deflate new object" or something, and
> not use the confusing (invented?) word "streamingly".
>
>> + the_hash_algo->final_oid_fn(¶no_oid, &c);
>> +
>> + close_loose_object(fd);
>> +
>> + oidcpy(oid, ¶no_oid);
>
> I see there's still quite a bit of duplication between this and
> write_loose_object(), but maybe it's not easy to factor out.
For what it's worth I tried to do that and the result doesn't really
seem worth it. I.e. something like the below. The inner loop of the
do/while looks like it could get a similar treatment, but likewise
doesn't seem worth the effort.
diff --git a/object-file.c b/object-file.c
index b0dea96906e..7fc2363cfa1 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1957,6 +1957,46 @@ static void setup_stream_and_header(git_zstream *stream,
the_hash_algo->update_fn(c, hdr, hdrlen);
}
+static int start_loose_object_common(struct strbuf *tmp_file,
+ const char *filename, unsigned flags,
+ git_zstream *stream,
+ unsigned char *buf, size_t buflen,
+ git_hash_ctx *c,
+ enum object_type type, size_t len,
+ char *hdr, int *hdrlen)
+{
+ int fd;
+
+ fd = create_tmpfile(tmp_file, filename, flags);
+ if (fd < 0)
+ return -1;
+
+ if (type != OBJ_NONE)
+ *hdrlen = format_object_header(hdr, *hdrlen, type, len);
+
+ /* Set it up and write header */
+ setup_stream_and_header(stream, buf, buflen, c, hdr, *hdrlen);
+
+ return fd;
+
+}
+
+static void end_loose_object_common(int ret, git_hash_ctx *c,
+ git_zstream *stream,
+ struct object_id *parano_oid,
+ const struct object_id *expected_oid,
+ const char *zstream_end_fmt,
+ const char *z_ok_fmt)
+{
+ if (ret != Z_STREAM_END)
+ die(_(zstream_end_fmt), ret, expected_oid);
+ ret = git_deflate_end_gently(stream);
+ if (ret != Z_OK)
+ die(_(z_ok_fmt), ret, expected_oid);
+ the_hash_algo->final_oid_fn(parano_oid, c);
+}
+
+
static int write_loose_object(const struct object_id *oid, char *hdr,
int hdrlen, const void *buf, unsigned long len,
time_t mtime, unsigned flags)
@@ -1970,15 +2010,12 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
static struct strbuf filename = STRBUF_INIT;
loose_object_path(the_repository, &filename, oid);
-
- fd = create_tmpfile(&tmp_file, filename.buf, flags);
+ fd = start_loose_object_common(&tmp_file, filename.buf, flags,
+ &stream, compressed, sizeof(compressed),
+ &c, OBJ_NONE, 0, hdr, &hdrlen);
if (fd < 0)
return -1;
- /* Set it up and write header */
- setup_stream_and_header(&stream, compressed, sizeof(compressed),
- &c, hdr, hdrlen);
-
/* Then the data itself.. */
stream.next_in = (void *)buf;
stream.avail_in = len;
@@ -1992,14 +2029,9 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
stream.avail_out = sizeof(compressed);
} while (ret == Z_OK);
- if (ret != Z_STREAM_END)
- die(_("unable to deflate new object %s (%d)"), oid_to_hex(oid),
- ret);
- ret = git_deflate_end_gently(&stream);
- if (ret != Z_OK)
- die(_("deflateEnd on object %s failed (%d)"), oid_to_hex(oid),
- ret);
- the_hash_algo->final_oid_fn(¶no_oid, &c);
+ end_loose_object_common(ret, &c, &stream, ¶no_oid, oid,
+ N_("unable to deflate new object %s (%d)"),
+ N_("deflateEnd on object %s failed (%d)"));
if (!oideq(oid, ¶no_oid))
die(_("confused by unstable object source data for %s"),
oid_to_hex(oid));
@@ -2049,16 +2081,12 @@ int write_stream_object_file(struct input_stream *in_stream, size_t len,
/* When oid is not determined, save tmp file to odb path. */
strbuf_addf(&filename, "%s/", get_object_directory());
- fd = create_tmpfile(&tmp_file, filename.buf, flags);
+ fd = start_loose_object_common(&tmp_file, filename.buf, flags,
+ &stream, compressed, sizeof(compressed),
+ &c, type, len, hdr, &hdrlen);
if (fd < 0)
return -1;
- hdrlen = format_object_header(hdr, hdrlen, type, len);
-
- /* Set it up and write header */
- setup_stream_and_header(&stream, compressed, sizeof(compressed),
- &c, hdr, hdrlen);
-
/* Then the data itself.. */
do {
unsigned char *in0 = stream.next_in;
@@ -2078,12 +2106,9 @@ int write_stream_object_file(struct input_stream *in_stream, size_t len,
stream.avail_out = sizeof(compressed);
} while (ret == Z_OK || ret == Z_BUF_ERROR);
- if (ret != Z_STREAM_END)
- die(_("unable to deflate new object streamingly (%d)"), ret);
- ret = git_deflate_end_gently(&stream);
- if (ret != Z_OK)
- die(_("deflateEnd on object streamingly failed (%d)"), ret);
- the_hash_algo->final_oid_fn(¶no_oid, &c);
+ end_loose_object_common(ret, &c, &stream, ¶no_oid, NULL,
+ N_("unable to deflate new object streamingly (%d)"),
+ N_("deflateEnd on object streamingly failed (%d)"));
close_loose_object(fd);
