object-file.c: simplify unpack_loose_short_header()

Combine the unpack_loose_short_header(),
unpack_loose_header_to_strbuf() and unpack_loose_header() functions
into one.

The unpack_loose_header_to_strbuf() function was added in
46f034483eb (sha1_file: support reading from a loose object of unknown
type, 2015-05-03).

Its code was mostly copy/pasted between it and both of
unpack_loose_header() and unpack_loose_short_header(). We now have a
single unpack_loose_header() function which accepts an optional
"struct strbuf *" instead.

I think the remaining unpack_loose_header() function could be further
simplified, we're carrying some complexity just to be able to emit a
garbage type longer than MAX_HEADER_LEN, we could alternatively just
say "we found a garbage type <first 32 bytes>..." instead. But let's
leave the current behavior in place for now.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/cache.h b/cache.h
index 985caced24668395a2531e78f714b8343feea4e7..11d2482e3099bcefeb9e0b2ab01d145e9d46fb82 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -1302,7 +1302,22 @@ char *xdg_cache_home(const char *filename);
 
 int git_open_cloexec(const char *name, int flags);
 #define git_open(name) git_open_cloexec(name, O_RDONLY)
-int unpack_loose_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz);
+
+/**
+ * unpack_loose_header() initializes the data stream needed to unpack
+ * a loose object header.
+ *
+ * Returns 0 on success. Returns negative values on error.
+ *
+ * It will only parse up to MAX_HEADER_LEN bytes unless an optional
+ * "hdrbuf" argument is non-NULL. This is intended for use with
+ * OBJECT_INFO_ALLOW_UNKNOWN_TYPE to extract the bad type for (error)
+ * reporting. The full header will be extracted to "hdrbuf" for use
+ * with parse_loose_header().
+ */
+int unpack_loose_header(git_zstream *stream, unsigned char *map,
+                       unsigned long mapsize, void *buffer,
+                       unsigned long bufsiz, struct strbuf *hdrbuf);
 struct object_info;
 int parse_loose_header(const char *hdr, struct object_info *oi,
                       unsigned int flags);

diff --git a/object-file.c b/object-file.c
index 6d97a6f69baf2f5a523b6fffcfcf5dd36e60902c..59b38aac2b83d452b049e063bf0d80d9bc51f787 100644 (file)
--- a/object-file.c
+++ b/object-file.c
@@ -1210,11 +1210,12 @@ void *map_loose_object(struct repository *r,
        return map_loose_object_1(r, NULL, oid, size);
 }
 
-static int unpack_loose_short_header(git_zstream *stream,
-                                    unsigned char *map, unsigned long mapsize,
-                                    void *buffer, unsigned long bufsiz)
+int unpack_loose_header(git_zstream *stream,
+                       unsigned char *map, unsigned long mapsize,
+                       void *buffer, unsigned long bufsiz,
+                       struct strbuf *header)
 {
-       int ret;
+       int status;
 
        /* Get the data stream */
        memset(stream, 0, sizeof(*stream));
@@ -1225,35 +1226,8 @@ static int unpack_loose_short_header(git_zstream *stream,
 
        git_inflate_init(stream);
        obj_read_unlock();
-       ret = git_inflate(stream, 0);
+       status = git_inflate(stream, 0);
        obj_read_lock();
-
-       return ret;
-}
-
-int unpack_loose_header(git_zstream *stream,
-                       unsigned char *map, unsigned long mapsize,
-                       void *buffer, unsigned long bufsiz)
-{
-       int status = unpack_loose_short_header(stream, map, mapsize,
-                                              buffer, bufsiz);
-
-       if (status < Z_OK)
-               return -1;
-
-       /* Make sure we have the terminating NUL */
-       if (!memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
-               return -1;
-       return 0;
-}
-
-static int unpack_loose_header_to_strbuf(git_zstream *stream, unsigned char *map,
-                                        unsigned long mapsize, void *buffer,
-                                        unsigned long bufsiz, struct strbuf *header)
-{
-       int status;
-
-       status = unpack_loose_short_header(stream, map, mapsize, buffer, bufsiz);
        if (status < Z_OK)
                return -1;
 
@@ -1263,6 +1237,14 @@ static int unpack_loose_header_to_strbuf(git_zstream *stream, unsigned char *map
        if (memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
                return 0;
 
+       /*
+        * We have a header longer than MAX_HEADER_LEN. The "header"
+        * here is only non-NULL when we run "cat-file
+        * --allow-unknown-type".
+        */
+       if (!header)
+               return -1;
+
        /*
         * buffer[0..bufsiz] was not large enough.  Copy the partial
         * result out to header, and then append the result of further
@@ -1412,6 +1394,7 @@ static int loose_object_info(struct repository *r,
        char hdr[MAX_HEADER_LEN];
        struct strbuf hdrbuf = STRBUF_INIT;
        unsigned long size_scratch;
+       int allow_unknown = flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE;
 
        if (oi->delta_base_oid)
                oidclr(oi->delta_base_oid);
@@ -1445,11 +1428,9 @@ static int loose_object_info(struct repository *r,
 
        if (oi->disk_sizep)
                *oi->disk_sizep = mapsize;
-       if ((flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE)) {
-               if (unpack_loose_header_to_strbuf(&stream, map, mapsize, hdr, sizeof(hdr), &hdrbuf) < 0)
-                       status = error(_("unable to unpack %s header with --allow-unknown-type"),
-                                      oid_to_hex(oid));
-       } else if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
+
+       if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr),
+                               allow_unknown ? &hdrbuf : NULL) < 0)
                status = error(_("unable to unpack %s header"),
                               oid_to_hex(oid));
        if (status < 0)
@@ -2550,7 +2531,8 @@ int read_loose_object(const char *path,
                goto out;
        }
 
-       if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0) {
+       if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr),
+                               NULL) < 0) {
                error(_("unable to unpack header of %s"), path);
                goto out;
        }

diff --git a/streaming.c b/streaming.c
index 8beac62cbb71038a34096acd6f6c1a9b07723779..cb3c3cf6ff62f6097bfb05f3c72eaa1002ba2ffd 100644 (file)
--- a/streaming.c
+++ b/streaming.c
@@ -233,7 +233,8 @@ static int open_istream_loose(struct git_istream *st, struct repository *r,
                                 st->u.loose.mapped,
                                 st->u.loose.mapsize,
                                 st->u.loose.hdr,
-                                sizeof(st->u.loose.hdr)) < 0) ||
+                                sizeof(st->u.loose.hdr),
+                                NULL) < 0) ||
            (parse_loose_header(st->u.loose.hdr, &oi, 0) < 0)) {
                git_inflate_end(&st->z);
                munmap(st->u.loose.mapped, st->u.loose.mapsize);