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>

Ævar Arnfjörð Bjarmason committed Oct 1, 2021 at 11:16 UTC 01cab9767929c6c3faf4f4ad3b348639655f04fd
3 files changed +38 -40
cache.h
+16 -1
@@ -1302,7 +1302,22 @@ char *xdg_cache_home(const char *filename);
1302
1303 int git_open_cloexec(const char *name, int flags);
1304 #define git_open(name) git_open_cloexec(name, O_RDONLY)
1305 -int unpack_loose_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz);
1305 +
1306 +/**
1307 + * unpack_loose_header() initializes the data stream needed to unpack
1308 + * a loose object header.
1309 + *
1310 + * Returns 0 on success. Returns negative values on error.
1311 + *
1312 + * It will only parse up to MAX_HEADER_LEN bytes unless an optional
1313 + * "hdrbuf" argument is non-NULL. This is intended for use with
1314 + * OBJECT_INFO_ALLOW_UNKNOWN_TYPE to extract the bad type for (error)
1315 + * reporting. The full header will be extracted to "hdrbuf" for use
1316 + * with parse_loose_header().
1317 + */
1318 +int unpack_loose_header(git_zstream *stream, unsigned char *map,
1319 + unsigned long mapsize, void *buffer,
1320 + unsigned long bufsiz, struct strbuf *hdrbuf);
1321 struct object_info;
1322 int parse_loose_header(const char *hdr, struct object_info *oi,
1323 unsigned int flags);
object-file.c
+20 -38
@@ -1210,11 +1210,12 @@ void *map_loose_object(struct repository *r,
1210 return map_loose_object_1(r, NULL, oid, size);
1211 }
1212
1213 -static int unpack_loose_short_header(git_zstream *stream,
1214 - unsigned char *map, unsigned long mapsize,
1215 - void *buffer, unsigned long bufsiz)
1213 +int unpack_loose_header(git_zstream *stream,
1214 + unsigned char *map, unsigned long mapsize,
1215 + void *buffer, unsigned long bufsiz,
1216 + struct strbuf *header)
1217 {
1217 - int ret;
1218 + int status;
1219
1220 /* Get the data stream */
1221 memset(stream, 0, sizeof(*stream));
@@ -1225,35 +1226,8 @@ static int unpack_loose_short_header(git_zstream *stream,
1226
1227 git_inflate_init(stream);
1228 obj_read_unlock();
1228 - ret = git_inflate(stream, 0);
1229 + status = git_inflate(stream, 0);
1230 obj_read_lock();
1230 -
1231 - return ret;
1232 -}
1233 -
1234 -int unpack_loose_header(git_zstream *stream,
1235 - unsigned char *map, unsigned long mapsize,
1236 - void *buffer, unsigned long bufsiz)
1237 -{
1238 - int status = unpack_loose_short_header(stream, map, mapsize,
1239 - buffer, bufsiz);
1240 -
1241 - if (status < Z_OK)
1242 - return -1;
1243 -
1244 - /* Make sure we have the terminating NUL */
1245 - if (!memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
1246 - return -1;
1247 - return 0;
1248 -}
1249 -
1250 -static int unpack_loose_header_to_strbuf(git_zstream *stream, unsigned char *map,
1251 - unsigned long mapsize, void *buffer,
1252 - unsigned long bufsiz, struct strbuf *header)
1253 -{
1254 - int status;
1255 -
1256 - status = unpack_loose_short_header(stream, map, mapsize, buffer, bufsiz);
1231 if (status < Z_OK)
1232 return -1;
1233
@@ -1263,6 +1237,14 @@ static int unpack_loose_header_to_strbuf(git_zstream *stream, unsigned char *map
1237 if (memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
1238 return 0;
1239
1240 + /*
1241 + * We have a header longer than MAX_HEADER_LEN. The "header"
1242 + * here is only non-NULL when we run "cat-file
1243 + * --allow-unknown-type".
1244 + */
1245 + if (!header)
1246 + return -1;
1247 +
1248 /*
1249 * buffer[0..bufsiz] was not large enough. Copy the partial
1250 * result out to header, and then append the result of further
@@ -1412,6 +1394,7 @@ static int loose_object_info(struct repository *r,
1394 char hdr[MAX_HEADER_LEN];
1395 struct strbuf hdrbuf = STRBUF_INIT;
1396 unsigned long size_scratch;
1397 + int allow_unknown = flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE;
1398
1399 if (oi->delta_base_oid)
1400 oidclr(oi->delta_base_oid);
@@ -1445,11 +1428,9 @@ static int loose_object_info(struct repository *r,
1428
1429 if (oi->disk_sizep)
1430 *oi->disk_sizep = mapsize;
1448 - if ((flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE)) {
1449 - if (unpack_loose_header_to_strbuf(&stream, map, mapsize, hdr, sizeof(hdr), &hdrbuf) < 0)
1450 - status = error(_("unable to unpack %s header with --allow-unknown-type"),
1451 - oid_to_hex(oid));
1452 - } else if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
1431 +
1432 + if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr),
1433 + allow_unknown ? &hdrbuf : NULL) < 0)
1434 status = error(_("unable to unpack %s header"),
1435 oid_to_hex(oid));
1436 if (status < 0)
@@ -2550,7 +2531,8 @@ int read_loose_object(const char *path,
2531 goto out;
2532 }
2533
2553 - if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0) {
2534 + if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr),
2535 + NULL) < 0) {
2536 error(_("unable to unpack header of %s"), path);
2537 goto out;
2538 }
streaming.c
+2 -1
@@ -233,7 +233,8 @@ static int open_istream_loose(struct git_istream *st, struct repository *r,
233 st->u.loose.mapped,
234 st->u.loose.mapsize,
235 st->u.loose.hdr,
236 - sizeof(st->u.loose.hdr)) < 0) ||
236 + sizeof(st->u.loose.hdr),
237 + NULL) < 0) ||
238 (parse_loose_header(st->u.loose.hdr, &oi, 0) < 0)) {
239 git_inflate_end(&st->z);
240 munmap(st->u.loose.mapped, st->u.loose.mapsize);