{"thread":{"id":"39000","subject":"[PATCH v7 0/4] cat-file: teach cat-file a '--literally' option","startedAt":"2015-04-04T05:41:24Z","lastAt":"2015-04-09T03:24:09Z","messageCount":19,"participants":["karthik nayak","Karthik Nayak","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":7,"patchTotal":4},"messages":[{"id":"258977","messageId":"551F7984.5070902@gmail.com","threadId":"39000","inReplyTo":null,"subject":"[PATCH v7 0/4] cat-file: teach cat-file a '--literally' option","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-04T05:41:24Z","receivedAt":"2015-04-04T05:41:24Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Changes made :\n\n* Fixed the problem with packed types, which failed a few tests.\n* Misspelled email.\n\nThanks for the suggestions and comments on version 6.\n"},{"id":"258978","messageId":"1428126162-18987-1-git-send-email-karthik.188@gmail.com","threadId":"39000","inReplyTo":"551F7984.5070902@gmail.com","subject":"[PATCH v7 1/4] sha1_file.c: support reading from a loose object of unknown type","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-04T05:42:42Z","receivedAt":"2015-04-04T05:42:42Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Update sha1_loose_object_info() to optionally allow it to read\nfrom a loose object file of unknown/bogus type; as the function\nusually returns the type of the object it read in the form of enum\nfor known types, add an optional \"typename\" field to receive the\nname of the type in textual form and a flag to indicate the reading\nof a loose object file of unknown/bogus type.\n\nAdd parse_sha1_header_extended() which acts as a wrapper around\nparse_sha1_header() allowing more information to be obtained.\n\nAdd unpack_sha1_header_to_strbuf() to unpack sha1 headers of\nunknown/corrupt objects which have a unknown sha1 header size to\na strbuf structure. This was written by Junio C Hamano but tested\nby me.\n\nHelped-by: Junio C Hamano \nHelped-by: Eric Sunshine \nSigned-off-by: Karthik Nayak \n---\n cache.h | 2 ++\n sha1_file.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++------------\n 2 files changed, 91 insertions(+), 22 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex c47323e..ea6faf0 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -881,6 +881,7 @@ extern int is_ntfs_dotgit(const char *name);\n \n /* object replacement */\n #define LOOKUP_REPLACE_OBJECT 1\n+#define LOOKUP_LITERALLY 2\n extern void *read_sha1_file_extended(const unsigned char *sha1, enum object_type *type, unsigned long *size, unsigned flag);\n static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size)\n {\n@@ -1349,6 +1350,7 @@ struct object_info {\n \tunsigned long *sizep;\n \tunsigned long *disk_sizep;\n \tunsigned char *delta_base_sha1;\n+\tstruct strbuf *typename;\n \n \t/* Response */\n \tenum {\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 980ce6b..5a1b904 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1564,6 +1564,34 @@ int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long ma\n \treturn git_inflate(stream, 0);\n }\n \n+static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,\n+\t\t\t\t\tunsigned long mapsize,\n+\t\t\t\t\tstruct strbuf *header)\n+{\n+\tunsigned char buffer[32], *cp;\n+\tunsigned long bufsiz = sizeof(buffer);\n+\tint status;\n+\n+\tstatus = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);\n+\n+\tif (status) {\n+\t\tstrbuf_add(header, buffer, stream->next_out - buffer);\n+\t\treturn status;\n+\t}\n+\n+\tdo {\n+\t\tstatus = git_inflate(stream, 0);\n+\t\tstrbuf_add(header, buffer, stream->next_out - buffer);\n+\t\tfor (cp = buffer; cp < stream->next_out; cp++)\n+\t\t\tif (!*cp)\n+\t\t\t\t/* Found the NUL at the end of the header */\n+\t\t\t\treturn 0;\n+\t\tstream->next_out = buffer;\n+\t\tstream->avail_out = bufsiz;\n+\t} while (status != Z_STREAM_END);\n+\treturn -1;\n+}\n+\n static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long size, const unsigned char *sha1)\n {\n \tint bytes = strlen(buffer) + 1;\n@@ -1614,27 +1642,24 @@ static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long s\n * too permissive for what we want to check. So do an anal\n * object header parse by hand.\n */\n-int parse_sha1_header(const char *hdr, unsigned long *sizep)\n+int parse_sha1_header_extended(const char *hdr, struct object_info *oi,\n+\t\t\t unsigned int flags)\n {\n-\tchar type[10];\n-\tint i;\n+\tstruct strbuf typename = STRBUF_INIT;\n \tunsigned long size;\n+\tint type;\n \n \t/*\n \t * The type can be at most ten bytes (including the\n \t * terminating '\\0' that we add), and is followed by\n \t * a space.\n \t */\n-\ti = 0;\n \tfor (;;) {\n \t\tchar c = *hdr++;\n \t\tif (c == ' ')\n \t\t\tbreak;\n-\t\ttype[i++] = c;\n-\t\tif (i >= sizeof(type))\n-\t\t\treturn -1;\n+\t\tstrbuf_addch(&typename, c);\n \t}\n-\ttype[i] = 0;\n \n \t/*\n \t * The length must follow immediately, and be in canonical\n@@ -1652,12 +1677,39 @@ int parse_sha1_header(const char *hdr, unsigned long *sizep)\n \t\t\tsize = size * 10 + c;\n \t\t}\n \t}\n-\t*sizep = size;\n \n+\ttype = type_from_string_gently(typename.buf, typename.len, 1);\n+\tif (oi->sizep)\n+\t\t*oi->sizep = size;\n+\tif (oi->typename)\n+\t\tstrbuf_addbuf(oi->typename, &typename);\n+\tstrbuf_release(&typename);\n+\n+\t/*\n+\t * Set type to 0 if its an unknown object and\n+\t * we're obtaining the type using '--literally'\n+\t * option.\n+\t */\n+\tif ((flags & LOOKUP_LITERALLY) && (type == -1))\n+\t\ttype = 0;\n+\telse if (type == -1)\n+\t\tdie(\"invalid object type\");\n+\tif (oi->typep)\n+\t\t*oi->typep = type;\n \t/*\n \t * The length must be followed by a zero byte\n \t */\n-\treturn *hdr ? -1 : type_from_string(type);\n+\treturn *hdr ? -1 : type;\n+}\n+\n+int parse_sha1_header(const char *hdr, unsigned long *sizep)\n+{\n+\tstruct object_info oi;\n+\n+\toi.sizep = sizep;\n+\toi.typename = NULL;\n+\toi.typep = NULL;\n+\treturn parse_sha1_header_extended(hdr, &oi, LOOKUP_REPLACE_OBJECT);\n }\n \n static void *unpack_sha1_file(void *map, unsigned long mapsize, enum object_type *type, unsigned long *size, const unsigned char *sha1)\n@@ -2522,13 +2574,15 @@ struct packed_git *find_sha1_pack(const unsigned char *sha1,\n }\n \n static int sha1_loose_object_info(const unsigned char *sha1,\n-\t\t\t\t struct object_info *oi)\n+\t\t\t\t struct object_info *oi,\n+\t\t\t\t int flags)\n {\n-\tint status;\n-\tunsigned long mapsize, size;\n+\tint status = 0;\n+\tunsigned long mapsize;\n \tvoid *map;\n \tgit_zstream stream;\n \tchar hdr[32];\n+\tstruct strbuf hdrbuf = STRBUF_INIT;\n \n \tif (oi->delta_base_sha1)\n \t\thashclr(oi->delta_base_sha1);\n@@ -2555,17 +2609,26 @@ static int sha1_loose_object_info(const unsigned char *sha1,\n \t\treturn -1;\n \tif (oi->disk_sizep)\n \t\t*oi->disk_sizep = mapsize;\n-\tif (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)\n-\t\tstatus = error(\"unable to unpack %s header\",\n-\t\t\t sha1_to_hex(sha1));\n-\telse if ((status = parse_sha1_header(hdr, &size)) < 0)\n-\t\tstatus = error(\"unable to parse %s header\", sha1_to_hex(sha1));\n-\telse if (oi->sizep)\n-\t\t*oi->sizep = size;\n+\tif ((flags & LOOKUP_LITERALLY)) {\n+\t\tif (unpack_sha1_header_to_strbuf(&stream, map, mapsize, &hdrbuf) < 0)\n+\t\t\tstatus = error(\"unable to unpack %s header with --literally\",\n+\t\t\t\t sha1_to_hex(sha1));\n+\t\telse if ((status = parse_sha1_header_extended(hdrbuf.buf, oi, flags)) < 0)\n+\t\t\tstatus = error(\"unable to parse %s header with --literally\",\n+\t\t\t\t sha1_to_hex(sha1));\n+\t} else {\n+\t\tif (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)\n+\t\t\tstatus = error(\"unable to unpack %s header\",\n+\t\t\t\t sha1_to_hex(sha1));\n+\t\telse if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 0)\n+\t\t\tstatus = error(\"unable to parse %s header\", sha1_to_hex(sha1));\n+\t}\n \tgit_inflate_end(&stream);\n \tmunmap(map, mapsize);\n-\tif (oi->typep)\n+\tif (status && oi->typep)\n \t\t*oi->typep = status;\n+\tif (hdrbuf.buf)\n+\t\tstrbuf_release(&hdrbuf);\n \treturn 0;\n }\n \n@@ -2586,13 +2649,15 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \t\t\t*(oi->disk_sizep) = 0;\n \t\tif (oi->delta_base_sha1)\n \t\t\thashclr(oi->delta_base_sha1);\n+\t\tif (oi->typename)\n+\t\t\tstrbuf_addstr(oi->typename, typename(co->type));\n \t\toi->whence = OI_CACHED;\n \t\treturn 0;\n \t}\n \n \tif (!find_pack_entry(real, &e)) {\n \t\t/* Most likely it's a loose object. */\n-\t\tif (!sha1_loose_object_info(real, oi)) {\n+\t\tif (!sha1_loose_object_info(real, oi, flags)) {\n \t\t\toi->whence = OI_LOOSE;\n \t\t\treturn 0;\n \t\t}\n@@ -2616,6 +2681,8 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \t\toi->u.packed.is_delta = (rtype == OBJ_REF_DELTA ||\n \t\t\t\t\t rtype == OBJ_OFS_DELTA);\n \t}\n+\tif (oi->typename)\n+\t\tstrbuf_addstr(oi->typename, typename(*oi->typep));\n \n \treturn 0;\n }\n-- \n2.4.0.rc1.249.g9f2ee54\n"},{"id":"258979","messageId":"1428126244-19115-1-git-send-email-karthik.188@gmail.com","threadId":"39000","inReplyTo":"551F7984.5070902@gmail.com","subject":"[PATCH v7 2/4] cat-file: teach cat-file a '--literally' option","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-04T05:44:04Z","receivedAt":"2015-04-04T05:44:04Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Currently 'git cat-file' throws an error while trying to\nprint the type or size of a broken/corrupt object which is\ncreated using 'git hash-object --literally'. This is\nbecause these objects are usually of unknown types.\n\nTeach git cat-file a '--literally' option where it prints\nthe type or size of a broken/corrupt object without throwing\nan error.\n\nModify '-t' and '-s' options to call sha1_object_info_extended()\ndirectly to support the '--literally' option.\n\nHelped-by: Junio C Hamano \nHelped-by: Eric Sunshine \nSigned-off-by: Karthik Nayak \n---\n builtin/cat-file.c | 38 +++++++++++++++++++++++++++++---------\n 1 file changed, 29 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex df99df4..91ceae0 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -9,13 +9,20 @@\n #include \"userdiff.h\"\n #include \"streaming.h\"\n \n-static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n+static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n+\t\t\tint literally)\n {\n \tunsigned char sha1[20];\n \tenum object_type type;\n \tchar *buf;\n \tunsigned long size;\n \tstruct object_context obj_context;\n+\tstruct object_info oi = {NULL};\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tunsigned flags = LOOKUP_REPLACE_OBJECT;\n+\n+\tif (literally)\n+\t\tflags |= LOOKUP_LITERALLY;\n \n \tif (get_sha1_with_context(obj_name, 0, sha1, &obj_context))\n \t\tdie(\"Not a valid object name %s\", obj_name);\n@@ -23,16 +30,24 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n \tbuf = NULL;\n \tswitch (opt) {\n \tcase 't':\n-\t\ttype = sha1_object_info(sha1, NULL);\n-\t\tif (type > 0) {\n-\t\t\tprintf(\"%s\\n\", typename(type));\n+\t\toi.typep = &type;\n+\t\toi.typename = &sb;\n+\t\tif (sha1_object_info_extended(sha1, &oi, flags) < 0)\n+\t\t\tdie(\"git cat-file: could not get object info\");\n+\t\tif (type >= 0 && sb.len) {\n+\t\t\tprintf(\"%s\\n\", sb.buf);\n+\t\t\tstrbuf_release(&sb);\n \t\t\treturn 0;\n \t\t}\n \t\tbreak;\n \n \tcase 's':\n-\t\ttype = sha1_object_info(sha1, &size);\n-\t\tif (type > 0) {\n+\t\toi.typep = &type;\n+\t\toi.typename = &sb;\n+\t\toi.sizep = &size;\n+\t\tif (sha1_object_info_extended(sha1, &oi, flags) < 0)\n+\t\t\tdie(\"git cat-file: could not get object info\");\n+\t\tif (type >= 0 && sb.len) {\n \t\t\tprintf(\"%lu\\n\", size);\n \t\t\treturn 0;\n \t\t}\n@@ -323,7 +338,7 @@ static int batch_objects(struct batch_options *opt)\n }\n \n static const char * const cat_file_usage[] = {\n-\tN_(\"git cat-file (-t | -s | -e | -p | | --textconv) \"),\n+\tN_(\"git cat-file (-t [--literally]|-s [--literally]|-e|-p||--textconv) \"),\n \tN_(\"git cat-file (--batch | --batch-check) < \"),\n \tNULL\n };\n@@ -359,6 +374,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \tint opt = 0;\n \tconst char *exp_type = NULL, *obj_name = NULL;\n \tstruct batch_options batch = {0};\n+\tint literally = 0;\n \n \tconst struct option options[] = {\n \t\tOPT_GROUP(N_(\" can be one of: blob, tree, commit, tag\")),\n@@ -369,6 +385,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \t\tOPT_SET_INT('p', NULL, &opt, N_(\"pretty-print object's content\"), 'p'),\n \t\tOPT_SET_INT(0, \"textconv\", &opt,\n \t\t\t N_(\"for blob objects, run textconv on object's content\"), 'c'),\n+\t\tOPT_BOOL( 0, \"literally\", &literally,\n+\t\t\t N_(\"get information about corrupt objects for debugging Git\")),\n \t\t{ OPTION_CALLBACK, 0, \"batch\", &batch, \"format\",\n \t\t\tN_(\"show info and content of objects fed from the standard input\"),\n \t\t\tPARSE_OPT_OPTARG, batch_option_callback },\n@@ -380,7 +398,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \n \tgit_config(git_cat_file_config, NULL);\n \n-\tif (argc != 3 && argc != 2)\n+\tif (argc < 2 || argc > 4)\n \t\tusage_with_options(cat_file_usage, options);\n \n \targc = parse_options(argc, argv, prefix, options, cat_file_usage, 0);\n@@ -405,5 +423,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \tif (batch.enabled)\n \t\treturn batch_objects(&batch);\n \n-\treturn cat_one_file(opt, exp_type, obj_name);\n+\tif (literally && opt != 't' && opt != 's')\n+\t\tdie(\"git cat-file --literally: use with -s or -t\");\n+\treturn cat_one_file(opt, exp_type, obj_name, literally);\n }\n-- \n2.4.0.rc1.249.g9f2ee54\n"},{"id":"258980","messageId":"1428126264-19179-1-git-send-email-karthik.188@gmail.com","threadId":"39000","inReplyTo":"551F7984.5070902@gmail.com","subject":"[PATCH v7 3/4] cat-file: add documentation for '--literally' option.","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-04T05:44:24Z","receivedAt":"2015-04-04T05:44:24Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Signed-off-by: Karthik Nayak \n---\n Documentation/git-cat-file.txt | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex f6a16f4..8bac7bd 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -9,7 +9,7 @@ git-cat-file - Provide content or type and size information for repository objec\n SYNOPSIS\n --------\n [verse]\n-'git cat-file' (-t | -s | -e | -p | | --textconv ) \n+'git cat-file' (-t [--literally]| -s [--literally]| -e | -p | | --textconv ) \n 'git cat-file' (--batch | --batch-check) < \n \n DESCRIPTION\n@@ -69,6 +69,10 @@ OPTIONS\n \tnot be combined with any other options or arguments. See the\n \tsection `BATCH OUTPUT` below for details.\n \n+--literally::\n+\tPrint information of broken/corrupt objects of unknown type without\n+\tthrowing an error. To be used combined with '-s' or '-t' option.\n+\n OUTPUT\n ------\n If '-t' is specified, one of the .\n-- \n2.4.0.rc1.249.g9f2ee54\n"},{"id":"258981","messageId":"1428126289-19239-1-git-send-email-karthik.188@gmail.com","threadId":"39000","inReplyTo":"551F7984.5070902@gmail.com","subject":"[PATCH v7 4/4] t1006: add tests for git cat-file --literally","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-04T05:44:49Z","receivedAt":"2015-04-04T05:44:49Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Signed-off-by: Karthik Nayak \n---\n t/t1006-cat-file.sh | 27 +++++++++++++++++++++++++++\n 1 file changed, 27 insertions(+)\n\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex ab36b1e..5b74044 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -47,6 +47,18 @@ $content\"\n \ttest_cmp expect actual\n '\n \n+ test_expect_success \"Type of $type is correct using --literally\" '\n+\techo $type >expect &&\n+\tgit cat-file -t --literally $sha1 >actual &&\n+\ttest_cmp expect actual\n+ '\n+\n+ test_expect_success \"Size of $type is correct using --literally\" '\n+\techo $size >expect &&\n+\tgit cat-file -s --literally $sha1 >actual &&\n+\ttest_cmp expect actual\n+ '\n+\n test -z \"$content\" ||\n test_expect_success \"Content of $type is correct\" '\n \tmaybe_remove_timestamp \"$content\" $no_ts >expect &&\n@@ -296,4 +308,19 @@ test_expect_success '%(deltabase) reports packed delta bases' '\n \t}\n '\n \n+bogus_type=\"bogus\"\n+bogus_sha1=$(git hash-object -t $bogus_type --literally -w --stdin expect &&\n+\tgit cat-file -t --literally $bogus_sha1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success \"Size of broken object is correct\" '\n+\techo \"0\" >expect &&\n+\tgit cat-file -s --literally $bogus_sha1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.4.0.rc1.249.g9f2ee54\n"},{"id":"258995","messageId":"xmqq7ftrg02b.fsf@gitster.dls.corp.google.com","threadId":"39000","inReplyTo":"1428126162-18987-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v7 1/4] sha1_file.c: support reading from a loose object of unknown type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-04T19:34:04Z","receivedAt":"2015-04-04T19:34:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> @@ -2586,13 +2649,15 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n> \t\t\t*(oi->disk_sizep) = 0;\n> \t\tif (oi->delta_base_sha1)\n> \t\t\thashclr(oi->delta_base_sha1);\n> +\t\tif (oi->typename)\n> +\t\t\tstrbuf_addstr(oi->typename, typename(co->type));\n> \t\toi->whence = OI_CACHED;\n> \t\treturn 0;\n> \t}\n\nJust before the pre-context of this hunk, there is this bit:\n\n\tif (oi->typep)\n\t\t*(oi->typep) = co->type;\n\nwhich tells me that the callers of this function is allowed to pass\na NULL in oi->typep when they are not interested in the type of the\nobject.\n\n> \tif (!find_pack_entry(real, &e)) {\n> \t\t/* Most likely it's a loose object. */\n> -\t\tif (!sha1_loose_object_info(real, oi)) {\n> +\t\tif (!sha1_loose_object_info(real, oi, flags)) {\n> \t\t\toi->whence = OI_LOOSE;\n> \t\t\treturn 0;\n> \t\t}\n> @@ -2616,6 +2681,8 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n> \t\toi->u.packed.is_delta = (rtype == OBJ_REF_DELTA ||\n> \t\t\t\t\t rtype == OBJ_OFS_DELTA);\n> \t}\n> +\tif (oi->typename)\n> +\t\tstrbuf_addstr(oi->typename, typename(*oi->typep));\n\nSo, it makes me wonder what guarantee we have that this does not\ndereference a NULL here.\n"},{"id":"258997","messageId":"55204141.9070100@gmail.com","threadId":"39000","inReplyTo":"xmqq7ftrg02b.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v7 1/4] sha1_file.c: support reading from a loose object of unknown type","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-04T19:53:37Z","receivedAt":"2015-04-04T19:53:37Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\nOn 04/05/2015 01:04 AM, Junio C Hamano wrote:\n> Karthik Nayak writes:\n>\n> > @@ -2586,13 +2649,15 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n> > *(oi->disk_sizep) = 0;\n> > if (oi->delta_base_sha1)\n> > hashclr(oi->delta_base_sha1);\n> > + if (oi->typename)\n> > + strbuf_addstr(oi->typename, typename(co->type));\n> > oi->whence = OI_CACHED;\n> > return 0;\n> > }\n>\n> Just before the pre-context of this hunk, there is this bit:\n>\n> if (oi->typep)\n> *(oi->typep) = co->type;\n>\n> which tells me that the callers of this function is allowed to pass\n> a NULL in oi->typep when they are not interested in the type of the\n> object.\n>\n> > if (!find_pack_entry(real, &e)) {\n> > /* Most likely it's a loose object. */\n> > - if (!sha1_loose_object_info(real, oi)) {\n> > + if (!sha1_loose_object_info(real, oi, flags)) {\n> > oi->whence = OI_LOOSE;\n> > return 0;\n> > }\n> > @@ -2616,6 +2681,8 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n> > oi->u.packed.is_delta = (rtype == OBJ_REF_DELTA ||\n> > rtype == OBJ_OFS_DELTA);\n> > }\n> > + if (oi->typename)\n> > + strbuf_addstr(oi->typename, typename(*oi->typep));\n>\n> So, it makes me wonder what guarantee we have that this does not\n> dereference a NULL here.\n>\nAs per my code, oi->typename is only pointing to something when oi->typep\nis ( As oi->typename is currently only used in cat-file.c).\nBut what you're saying also is true, there is no other guarantee, as a user may\nset oi->typename to point to a struct strbuf and leave out oi->typep.\n\n if (oi->typename && oi->typep)\n strbuf_addstr(oi->typename, typename(*oi->typep));\n\nThis should suffice. Do you want me to re-roll this?\n"},{"id":"259015","messageId":"xmqqiodbdnkn.fsf@gitster.dls.corp.google.com","threadId":"39000","inReplyTo":"55204141.9070100@gmail.com","subject":"Re: [PATCH v7 1/4] sha1_file.c: support reading from a loose object of unknown type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-05T07:46:48Z","receivedAt":"2015-04-05T07:46:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"karthik nayak writes:\n\n>> So, it makes me wonder what guarantee we have that this does not\n>> dereference a NULL here.\n>>\n> As per my code, oi->typename is only pointing to something when oi->typep\n> is ( As oi->typename is currently only used in cat-file.c).\n> But what you're saying also is true, there is no other guarantee, as a user may\n> set oi->typename to point to a struct strbuf and leave out oi->typep.\n>\n> if (oi->typename && oi->typep)\n> strbuf_addstr(oi->typename, typename(*oi->typep));\n>\n> This should suffice. Do you want me to re-roll this?\n\nI'd rather avoid the thinking along the lines of \"at this moment,\nthere happens to be only one caller that asks for typename and the\ncaller also sets typep, so we will be safe as long as we make sure\nthe caller passed typep before giving him typename back\".\n\nSomebody else may write new code that wants to learn the typename,\nforgets to set typep, calls into this codepath, and ends up\nscratching his head wondering why the typename string is returned to\nhim. Surely the code may not crash at the new code you wrote, but\nyou are not helping him.\n\nIf it semantically does not make sense to ask for the typename\nwithout asking for the type code, then we can and should make that\nas a new calling convention _all_ callers must follow.\n\nIn other words, I think it is better to have\n\n\tif (oi->typename && !oi->typep)\n\t\tdie(\"BUG\");\n\nsomewhere near the beginning of the callchain that takes oi; that\nwill ensure all callers understand the rule.\n"},{"id":"259016","messageId":"5520E9D2.4010603@gmail.com","threadId":"39000","inReplyTo":"xmqqiodbdnkn.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v7 1/4] sha1_file.c: support reading from a loose object of unknown type","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-05T07:52:50Z","receivedAt":"2015-04-05T07:52:50Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\n\nOn 04/05/2015 01:16 PM, Junio C Hamano wrote:\n> karthik nayak writes:\n>\n>>> So, it makes me wonder what guarantee we have that this does not\n>>> dereference a NULL here.\n>>>\n>> As per my code, oi->typename is only pointing to something when oi->typep\n>> is ( As oi->typename is currently only used in cat-file.c).\n>> But what you're saying also is true, there is no other guarantee, as a user may\n>> set oi->typename to point to a struct strbuf and leave out oi->typep.\n>>\n>> if (oi->typename && oi->typep)\n>> strbuf_addstr(oi->typename, typename(*oi->typep));\n>>\n>> This should suffice. Do you want me to re-roll this?\n>\n> I'd rather avoid the thinking along the lines of \"at this moment,\n> there happens to be only one caller that asks for typename and the\n> caller also sets typep, so we will be safe as long as we make sure\n> the caller passed typep before giving him typename back\".\n>\n> Somebody else may write new code that wants to learn the typename,\n> forgets to set typep, calls into this codepath, and ends up\n> scratching his head wondering why the typename string is returned to\n> him. Surely the code may not crash at the new code you wrote, but\n> you are not helping him.\n\n\nYes! I do agree with you, If you read my previous reply, I did mention \nwhat you said about not having a guarantee that typep and typename are \nboth set and another user might have a bug with this.\n\n>\n> If it semantically does not make sense to ask for the typename\n> without asking for the type code, then we can and should make that\n> as a new calling convention _all_ callers must follow.\n>\n> In other words, I think it is better to have\n>\n> \tif (oi->typename && !oi->typep)\n> \t\tdie(\"BUG\");\n>\n> somewhere near the beginning of the callchain that takes oi; that\n> will ensure all callers understand the rule.\n>\n\nYes! this is a better approach as it will enforce that typep must be set \nwhen typename is set.\n"},{"id":"259027","messageId":"1428258505-25223-1-git-send-email-karthik.188@gmail.com","threadId":"39000","inReplyTo":"1428126162-18987-1-git-send-email-karthik.188@gmail.com","subject":"[PATCH v7 1/4] sha1_file.c: support reading from a loose object of unknown type","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-05T18:28:25Z","receivedAt":"2015-04-05T18:28:25Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Update sha1_loose_object_info() to optionally allow it to read\nfrom a loose object file of unknown/bogus type; as the function\nusually returns the type of the object it read in the form of enum\nfor known types, add an optional \"typename\" field to receive the\nname of the type in textual form and a flag to indicate the reading\nof a loose object file of unknown/bogus type.\n\nAdd parse_sha1_header_extended() which acts as a wrapper around\nparse_sha1_header() allowing more information to be obtained.\n\nAdd unpack_sha1_header_to_strbuf() to unpack sha1 headers of\nunknown/corrupt objects which have a unknown sha1 header size to\na strbuf structure. This was written by Junio C Hamano but tested\nby me.\n\nHelped-by: Junio C Hamano \nHelped-by: Eric Sunshine \nSigned-off-by: Karthik Nayak \n---\n cache.h | 2 ++\n sha1_file.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++------------\n 2 files changed, 94 insertions(+), 22 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex c47323e..ea6faf0 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -881,6 +881,7 @@ extern int is_ntfs_dotgit(const char *name);\n \n /* object replacement */\n #define LOOKUP_REPLACE_OBJECT 1\n+#define LOOKUP_LITERALLY 2\n extern void *read_sha1_file_extended(const unsigned char *sha1, enum object_type *type, unsigned long *size, unsigned flag);\n static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size)\n {\n@@ -1349,6 +1350,7 @@ struct object_info {\n \tunsigned long *sizep;\n \tunsigned long *disk_sizep;\n \tunsigned char *delta_base_sha1;\n+\tstruct strbuf *typename;\n \n \t/* Response */\n \tenum {\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 980ce6b..ac8fffc 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1564,6 +1564,34 @@ int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long ma\n \treturn git_inflate(stream, 0);\n }\n \n+static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,\n+\t\t\t\t\tunsigned long mapsize,\n+\t\t\t\t\tstruct strbuf *header)\n+{\n+\tunsigned char buffer[32], *cp;\n+\tunsigned long bufsiz = sizeof(buffer);\n+\tint status;\n+\n+\tstatus = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);\n+\n+\tif (status) {\n+\t\tstrbuf_add(header, buffer, stream->next_out - buffer);\n+\t\treturn status;\n+\t}\n+\n+\tdo {\n+\t\tstatus = git_inflate(stream, 0);\n+\t\tstrbuf_add(header, buffer, stream->next_out - buffer);\n+\t\tfor (cp = buffer; cp < stream->next_out; cp++)\n+\t\t\tif (!*cp)\n+\t\t\t\t/* Found the NUL at the end of the header */\n+\t\t\t\treturn 0;\n+\t\tstream->next_out = buffer;\n+\t\tstream->avail_out = bufsiz;\n+\t} while (status != Z_STREAM_END);\n+\treturn -1;\n+}\n+\n static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long size, const unsigned char *sha1)\n {\n \tint bytes = strlen(buffer) + 1;\n@@ -1614,27 +1642,24 @@ static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long s\n * too permissive for what we want to check. So do an anal\n * object header parse by hand.\n */\n-int parse_sha1_header(const char *hdr, unsigned long *sizep)\n+int parse_sha1_header_extended(const char *hdr, struct object_info *oi,\n+\t\t\t unsigned int flags)\n {\n-\tchar type[10];\n-\tint i;\n+\tstruct strbuf typename = STRBUF_INIT;\n \tunsigned long size;\n+\tint type;\n \n \t/*\n \t * The type can be at most ten bytes (including the\n \t * terminating '\\0' that we add), and is followed by\n \t * a space.\n \t */\n-\ti = 0;\n \tfor (;;) {\n \t\tchar c = *hdr++;\n \t\tif (c == ' ')\n \t\t\tbreak;\n-\t\ttype[i++] = c;\n-\t\tif (i >= sizeof(type))\n-\t\t\treturn -1;\n+\t\tstrbuf_addch(&typename, c);\n \t}\n-\ttype[i] = 0;\n \n \t/*\n \t * The length must follow immediately, and be in canonical\n@@ -1652,12 +1677,39 @@ int parse_sha1_header(const char *hdr, unsigned long *sizep)\n \t\t\tsize = size * 10 + c;\n \t\t}\n \t}\n-\t*sizep = size;\n+\n+\ttype = type_from_string_gently(typename.buf, typename.len, 1);\n+\tif (oi->sizep)\n+\t\t*oi->sizep = size;\n+\tif (oi->typename)\n+\t\tstrbuf_addbuf(oi->typename, &typename);\n+\tstrbuf_release(&typename);\n \n \t/*\n+\t * Set type to 0 if its an unknown object and\n+\t * we're obtaining the type using '--literally'\n+\t * option.\n+\t */\n+\tif ((flags & LOOKUP_LITERALLY) && (type == -1))\n+\t\ttype = 0;\n+\telse if (type == -1)\n+\t\tdie(\"invalid object type\");\n+\tif (oi->typep)\n+\t\t*oi->typep = type;\n+\t/*\n \t * The length must be followed by a zero byte\n \t */\n-\treturn *hdr ? -1 : type_from_string(type);\n+\treturn *hdr ? -1 : type;\n+}\n+\n+int parse_sha1_header(const char *hdr, unsigned long *sizep)\n+{\n+\tstruct object_info oi;\n+\n+\toi.sizep = sizep;\n+\toi.typename = NULL;\n+\toi.typep = NULL;\n+\treturn parse_sha1_header_extended(hdr, &oi, LOOKUP_REPLACE_OBJECT);\n }\n \n static void *unpack_sha1_file(void *map, unsigned long mapsize, enum object_type *type, unsigned long *size, const unsigned char *sha1)\n@@ -2522,13 +2574,15 @@ struct packed_git *find_sha1_pack(const unsigned char *sha1,\n }\n \n static int sha1_loose_object_info(const unsigned char *sha1,\n-\t\t\t\t struct object_info *oi)\n+\t\t\t\t struct object_info *oi,\n+\t\t\t\t int flags)\n {\n-\tint status;\n-\tunsigned long mapsize, size;\n+\tint status = 0;\n+\tunsigned long mapsize;\n \tvoid *map;\n \tgit_zstream stream;\n \tchar hdr[32];\n+\tstruct strbuf hdrbuf = STRBUF_INIT;\n \n \tif (oi->delta_base_sha1)\n \t\thashclr(oi->delta_base_sha1);\n@@ -2555,17 +2609,26 @@ static int sha1_loose_object_info(const unsigned char *sha1,\n \t\treturn -1;\n \tif (oi->disk_sizep)\n \t\t*oi->disk_sizep = mapsize;\n-\tif (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)\n-\t\tstatus = error(\"unable to unpack %s header\",\n-\t\t\t sha1_to_hex(sha1));\n-\telse if ((status = parse_sha1_header(hdr, &size)) < 0)\n-\t\tstatus = error(\"unable to parse %s header\", sha1_to_hex(sha1));\n-\telse if (oi->sizep)\n-\t\t*oi->sizep = size;\n+\tif ((flags & LOOKUP_LITERALLY)) {\n+\t\tif (unpack_sha1_header_to_strbuf(&stream, map, mapsize, &hdrbuf) < 0)\n+\t\t\tstatus = error(\"unable to unpack %s header with --literally\",\n+\t\t\t\t sha1_to_hex(sha1));\n+\t\telse if ((status = parse_sha1_header_extended(hdrbuf.buf, oi, flags)) < 0)\n+\t\t\tstatus = error(\"unable to parse %s header with --literally\",\n+\t\t\t\t sha1_to_hex(sha1));\n+\t} else {\n+\t\tif (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)\n+\t\t\tstatus = error(\"unable to unpack %s header\",\n+\t\t\t\t sha1_to_hex(sha1));\n+\t\telse if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 0)\n+\t\t\tstatus = error(\"unable to parse %s header\", sha1_to_hex(sha1));\n+\t}\n \tgit_inflate_end(&stream);\n \tmunmap(map, mapsize);\n-\tif (oi->typep)\n+\tif (status && oi->typep)\n \t\t*oi->typep = status;\n+\tif (hdrbuf.buf)\n+\t\tstrbuf_release(&hdrbuf);\n \treturn 0;\n }\n \n@@ -2576,6 +2639,9 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \tint rtype;\n \tconst unsigned char *real = lookup_replace_object_extended(sha1, flags);\n \n+\tif (oi->typename && !oi->typep)\n+\t\tdie(\"object_info: cannot use typename without typep\");\n+\n \tco = find_cached_object(real);\n \tif (co) {\n \t\tif (oi->typep)\n@@ -2586,13 +2652,15 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \t\t\t*(oi->disk_sizep) = 0;\n \t\tif (oi->delta_base_sha1)\n \t\t\thashclr(oi->delta_base_sha1);\n+\t\tif (oi->typename)\n+\t\t\tstrbuf_addstr(oi->typename, typename(co->type));\n \t\toi->whence = OI_CACHED;\n \t\treturn 0;\n \t}\n \n \tif (!find_pack_entry(real, &e)) {\n \t\t/* Most likely it's a loose object. */\n-\t\tif (!sha1_loose_object_info(real, oi)) {\n+\t\tif (!sha1_loose_object_info(real, oi, flags)) {\n \t\t\toi->whence = OI_LOOSE;\n \t\t\treturn 0;\n \t\t}\n@@ -2616,6 +2684,8 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \t\toi->u.packed.is_delta = (rtype == OBJ_REF_DELTA ||\n \t\t\t\t\t rtype == OBJ_OFS_DELTA);\n \t}\n+\tif (oi->typename)\n+\t\tstrbuf_addstr(oi->typename, typename(*oi->typep));\n \n \treturn 0;\n }\n-- \n2.4.0.rc1.249.g9f2ee54\n"},{"id":"259031","messageId":"xmqqbnj2e4ah.fsf@gitster.dls.corp.google.com","threadId":"39000","inReplyTo":"5520E9D2.4010603@gmail.com","subject":"Re: [PATCH v7 1/4] sha1_file.c: support reading from a loose object of unknown type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-05T19:57:58Z","receivedAt":"2015-04-05T19:57:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"karthik nayak writes:\n\n> On 04/05/2015 01:16 PM, Junio C Hamano wrote:\n>\n>> If it semantically does not make sense to ask for the typename\n>> without asking for the type code, then we can and should make that\n>> as a new calling convention _all_ callers must follow.\n>>\n>> In other words, I think it is better to have\n>>\n>> \tif (oi->typename && !oi->typep)\n>> \t\tdie(\"BUG\");\n>>\n>> somewhere near the beginning of the callchain that takes oi; that\n>> will ensure all callers understand the rule.\n>\n> Yes! this is a better approach as it will enforce that typep must be\n> set when typename is set.\n\nNot so fast ;-)\n\nThe key phrase in what I wrote above is \"If it does not make sense\",\nand I am not yet convinced if that is the case or not. If we are to\nchange the calling convention for the callers, the reason why it\ndoes not make sense to ask only for typename but not typep must be\nexplained in the log message.\n\nAnd if it turns out that the answer to that question is \"it is valid\nto ask only for typename\", then it would be wrong to force everybody\nwho wants to learn the stringified typename to supply typep. And in\nsuch a case it might be better to do something like this instead (on\ntop of your patch I am responding to):\n\n sha1_file.c | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex f4055dd..26fbb7c 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2639,6 +2639,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \tstruct cached_object *co;\n \tstruct pack_entry e;\n \tint rtype;\n+\tenum object_type real_type;\n \tconst unsigned char *real = lookup_replace_object_extended(sha1, flags);\n \n \tco = find_cached_object(real);\n@@ -2670,9 +2671,18 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \t\t\treturn -1;\n \t}\n \n+\t/*\n+\t * packed_object_info() does not follow the delta chain to\n+\t * find out the real type, unless it is given oi->typep.\n+\t */\n+\tif (oi->typename && !oi->typep)\n+\t\toi->typep = &real_type;\n+\n \trtype = packed_object_info(e.p, e.offset, oi);\n \tif (rtype < 0) {\n \t\tmark_bad_packed_object(e.p, real);\n+\t\tif (oi->typep == &real_type)\n+\t\t\toi->typep = NULL;\n \t\treturn sha1_object_info_extended(real, oi, 0);\n \t} else if (in_delta_base_cache(e.p, e.offset)) {\n \t\toi->whence = OI_DBCACHED;\n@@ -2686,6 +2696,8 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \tif (oi->typename)\n \t\tstrbuf_addstr(oi->typename, typename(*oi->typep));\n \n+\tif (oi->typep == &real_type)\n+\t\toi->typep = NULL;\n \treturn 0;\n }\n \n"},{"id":"259121","messageId":"5523B2A7.6060604@gmail.com","threadId":"39000","inReplyTo":"xmqqbnj2e4ah.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v7 1/4] sha1_file.c: support reading from a loose object of unknown type","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-07T10:34:15Z","receivedAt":"2015-04-07T10:34:15Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\nOn 04/06/2015 01:27 AM, Junio C Hamano wrote:\n> karthik nayak writes:\n>\n> > On 04/05/2015 01:16 PM, Junio C Hamano wrote:\n> >\n> >> If it semantically does not make sense to ask for the typename\n> >> without asking for the type code, then we can and should make that\n> >> as a new calling convention _all_ callers must follow.\n> >>\n> >> In other words, I think it is better to have\n> >>\n> >> if (oi->typename && !oi->typep)\n> >> die(\"BUG\");\n> >>\n> >> somewhere near the beginning of the callchain that takes oi; that\n> >> will ensure all callers understand the rule.\n> >\n> > Yes! this is a better approach as it will enforce that typep must be\n> > set when typename is set.\n>\n> Not so fast ;-)\n>\n> The key phrase in what I wrote above is \"If it does not make sense\",\n> and I am not yet convinced if that is the case or not. If we are to\n> change the calling convention for the callers, the reason why it\n> does not make sense to ask only for typename but not typep must be\n> explained in the log message.\n>\n> And if it turns out that the answer to that question is \"it is valid\n> to ask only for typename\", then it would be wrong to force everybody\n> who wants to learn the stringified typename to supply typep. And in\n> such a case it might be better to do something like this instead (on\n> top of your patch I am responding to):\n>\n> sha1_file.c | 12 ++++++++++++\n> 1 file changed, 12 insertions(+)\n>\n> diff --git a/sha1_file.c b/sha1_file.c\n> index f4055dd..26fbb7c 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -2639,6 +2639,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n> struct cached_object *co;\n> struct pack_entry e;\n> int rtype;\n> + enum object_type real_type;\n> const unsigned char *real = lookup_replace_object_extended(sha1, flags);\n>\n> co = find_cached_object(real);\n> @@ -2670,9 +2671,18 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n> return -1;\n> }\n>\n> + /*\n> + * packed_object_info() does not follow the delta chain to\n> + * find out the real type, unless it is given oi->typep.\n> + */\n> + if (oi->typename && !oi->typep)\n> + oi->typep = &real_type;\n> +\n> rtype = packed_object_info(e.p, e.offset, oi);\n> if (rtype < 0) {\n> mark_bad_packed_object(e.p, real);\n> + if (oi->typep == &real_type)\n> + oi->typep = NULL;\n> return sha1_object_info_extended(real, oi, 0);\n> } else if (in_delta_base_cache(e.p, e.offset)) {\n> oi->whence = OI_DBCACHED;\n> @@ -2686,6 +2696,8 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n> if (oi->typename)\n> strbuf_addstr(oi->typename, typename(*oi->typep));\n>\n> + if (oi->typep == &real_type)\n> + oi->typep = NULL;\n> return 0;\n> }\n>\n>\nHaha, If there is anything I love about code revisions its how you are subjected to\nso many different ways of thinking. I didn't think of what you said.\n\nWe could do this and support typename without typep being used. This could probably\neven eliminate the typep used by cat-file while getting the type of an object.\n\nWill go through this again.\n\nThanks a lot.\n"},{"id":"259153","messageId":"CAPig+cQfdvyscUoMxCk7JrvzqaKRGTg=6E+YUCLtciHr7Zq9SA@mail.gmail.com","threadId":"39000","inReplyTo":"1428258505-25223-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v7 1/4] sha1_file.c: support reading from a loose object of unknown type","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-04-07T20:46:33Z","receivedAt":"2015-04-07T20:46:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Apr 5, 2015 at 2:28 PM, Karthik Nayak wrote:\n> Update sha1_loose_object_info() to optionally allow it to read\n> from a loose object file of unknown/bogus type; as the function\n> usually returns the type of the object it read in the form of enum\n> for known types, add an optional \"typename\" field to receive the\n> name of the type in textual form and a flag to indicate the reading\n> of a loose object file of unknown/bogus type.\n>\n> Add parse_sha1_header_extended() which acts as a wrapper around\n> parse_sha1_header() allowing more information to be obtained.\n>\n> Add unpack_sha1_header_to_strbuf() to unpack sha1 headers of\n> unknown/corrupt objects which have a unknown sha1 header size to\n> a strbuf structure. This was written by Junio C Hamano but tested\n> by me.\n>\n> Helped-by: Junio C Hamano \n> Helped-by: Eric Sunshine \n> Signed-off-by: Karthik Nayak \n> ---\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 980ce6b..ac8fffc 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -1614,27 +1642,24 @@ static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long s\n> * too permissive for what we want to check. So do an anal\n> * object header parse by hand.\n> */\n> -int parse_sha1_header(const char *hdr, unsigned long *sizep)\n> +int parse_sha1_header_extended(const char *hdr, struct object_info *oi,\n> + unsigned int flags)\n> {\n> - char type[10];\n> - int i;\n> + struct strbuf typename = STRBUF_INIT;\n> unsigned long size;\n> + int type;\n>\n> /*\n> * The type can be at most ten bytes (including the\n> * terminating '\\0' that we add), and is followed by\n> * a space.\n> */\n> - i = 0;\n> for (;;) {\n> char c = *hdr++;\n> if (c == ' ')\n> break;\n> - type[i++] = c;\n> - if (i >= sizeof(type))\n> - return -1;\n> + strbuf_addch(&typename, c);\n> }\n> - type[i] = 0;\n>\n> /*\n> * The length must follow immediately, and be in canonical\n> @@ -1652,12 +1677,39 @@ int parse_sha1_header(const char *hdr, unsigned long *sizep)\n> size = size * 10 + c;\n> }\n> }\n> - *sizep = size;\n> +\n> + type = type_from_string_gently(typename.buf, typename.len, 1);\n> + if (oi->sizep)\n> + *oi->sizep = size;\n> + if (oi->typename)\n> + strbuf_addbuf(oi->typename, &typename);\n> + strbuf_release(&typename);\n>\n> /*\n> + * Set type to 0 if its an unknown object and\n> + * we're obtaining the type using '--literally'\n> + * option.\n> + */\n> + if ((flags & LOOKUP_LITERALLY) && (type == -1))\n> + type = 0;\n> + else if (type == -1)\n> + die(\"invalid object type\");\n> + if (oi->typep)\n> + *oi->typep = type;\n\nThis unnecessary intermixing of 'type'/'typename' and 'size'\nprocessing makes the code more confusing than it ought to be. Why not\ndo all the processing related to 'type'/'typename' before the\nprocessing of 'size'?\n\n> + /*\n> * The length must be followed by a zero byte\n> */\n> - return *hdr ? -1 : type_from_string(type);\n> + return *hdr ? -1 : type;\n> +}\n> +\n> +int parse_sha1_header(const char *hdr, unsigned long *sizep)\n> +{\n> + struct object_info oi;\n> +\n> + oi.sizep = sizep;\n> + oi.typename = NULL;\n> + oi.typep = NULL;\n> + return parse_sha1_header_extended(hdr, &oi, LOOKUP_REPLACE_OBJECT);\n> }\n>\n> static void *unpack_sha1_file(void *map, unsigned long mapsize, enum object_type *type, unsigned long *size, const unsigned char *sha1)\n"},{"id":"259154","messageId":"CAPig+cQ_EQYmP14+g=ozi1eiGUqkrVN3gX-J4zshLpqL20iRcA@mail.gmail.com","threadId":"39000","inReplyTo":"1428126244-19115-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v7 2/4] cat-file: teach cat-file a '--literally' option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-04-07T20:49:09Z","receivedAt":"2015-04-07T20:49:09Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 4, 2015 at 1:44 AM, Karthik Nayak wrote:\n> Currently 'git cat-file' throws an error while trying to\n> print the type or size of a broken/corrupt object which is\n> created using 'git hash-object --literally'. This is\n> because these objects are usually of unknown types.\n\nThis focus of this explanation is off-the-mark. The fact that such\nobjects can be created by 'hash-object --literally' is tangental to\nthe real purpose of the new 'cat-file --literally' option, which is\nthat it can help with diagnosing broken/corrupt objects encountered\nin-the-wild.\n\nEven mentioning 'hash-object --literally' here may be misleading and\nconfusing since its purpose it to intentionally create broken objects\nfor stress-testing git itself. I'd probably drop the reference\naltogether, but if you insist upon mentioning 'hash-object\n--literally', perhaps make it a very minor parenthetical comment at\nthe end of the commit message saying that 'cat-file --literally' was\ninspired by its hash-object counterpart, or some such.\n\nMore below.\n\n> Teach git cat-file a '--literally' option where it prints\n> the type or size of a broken/corrupt object without throwing\n> an error.\n>\n> Modify '-t' and '-s' options to call sha1_object_info_extended()\n> directly to support the '--literally' option.\n>\n> Helped-by: Junio C Hamano \n> Helped-by: Eric Sunshine \n> Signed-off-by: Karthik Nayak \n> ---\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index df99df4..91ceae0 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -9,13 +9,20 @@\n> #include \"userdiff.h\"\n> #include \"streaming.h\"\n>\n> -static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n> +static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n> + int literally)\n> {\n> unsigned char sha1[20];\n> enum object_type type;\n> char *buf;\n> unsigned long size;\n> struct object_context obj_context;\n> + struct object_info oi = {NULL};\n> + struct strbuf sb = STRBUF_INIT;\n> + unsigned flags = LOOKUP_REPLACE_OBJECT;\n> +\n> + if (literally)\n> + flags |= LOOKUP_LITERALLY;\n>\n> if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))\n> die(\"Not a valid object name %s\", obj_name);\n> @@ -23,16 +30,24 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n> buf = NULL;\n> switch (opt) {\n> case 't':\n> - type = sha1_object_info(sha1, NULL);\n> - if (type > 0) {\n> - printf(\"%s\\n\", typename(type));\n> + oi.typep = &type;\n> + oi.typename = &sb;\n\nThese two lines are common to the -t and -s cases. Would it make sense\nto instead move them to just after 'oi' and 'sb' are declared? However\n(see below)...\n\n> + if (sha1_object_info_extended(sha1, &oi, flags) < 0)\n> + die(\"git cat-file: could not get object info\");\n> + if (type >= 0 && sb.len) {\n> + printf(\"%s\\n\", sb.buf);\n> + strbuf_release(&sb);\n\nHere you release the strbuf...\n\n> return 0;\n> }\n> break;\n>\n> case 's':\n> - type = sha1_object_info(sha1, &size);\n> - if (type > 0) {\n> + oi.typep = &type;\n> + oi.typename = &sb;\n\nWhy do you need to collect 'typename' for the -s case?\nsha1_object_info_extended() promises that 'type' will be zero in the\n--literally case for unknown types, so checking 'sb.len' in the\nconditional below doesn't buy you anything, does it?\n\nIn fact, it's not even clear why you need to collect 'type' in the -s\ncase? The return value of sha1_object_info_extended() already tells\nyou whether or not the 'size' was retrieved successfully (--literally\nor not).\n\n> + oi.sizep = &size;\n> + if (sha1_object_info_extended(sha1, &oi, flags) < 0)\n> + die(\"git cat-file: could not get object info\");\n> + if (type >= 0 && sb.len) {\n> printf(\"%lu\\n\", size);\n\nBut here you do not release the strbuf.\n\n> return 0;\n> }\n> @@ -369,6 +385,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n> OPT_SET_INT('p', NULL, &opt, N_(\"pretty-print object's content\"), 'p'),\n> OPT_SET_INT(0, \"textconv\", &opt,\n> N_(\"for blob objects, run textconv on object's content\"), 'c'),\n> + OPT_BOOL( 0, \"literally\", &literally,\n> + N_(\"get information about corrupt objects for debugging Git\")),\n\nThis option neither \"gets information\" nor is it for debugging Git.\nRather, it's useful for diagnosing broken/corrupt objects in\ncombination with other options. Perhaps rephrase something like this:\n\n \"allow -s and -t to work with broken/corrupt objects\"\n\n> { OPTION_CALLBACK, 0, \"batch\", &batch, \"format\",\n> N_(\"show info and content of objects fed from the standard input\"),\n> PARSE_OPT_OPTARG, batch_option_callback },\n"},{"id":"259155","messageId":"CAPig+cRF7-qTby_s=vQPCTN7JYS9=8q3WO3zkbkGUP4Ax=o=wg@mail.gmail.com","threadId":"39000","inReplyTo":"1428126264-19179-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v7 3/4] cat-file: add documentation for '--literally' option.","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-04-07T20:49:30Z","receivedAt":"2015-04-07T20:49:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 4, 2015 at 1:44 AM, Karthik Nayak wrote:\n> Signed-off-by: Karthik Nayak \n> ---\n> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\n> index f6a16f4..8bac7bd 100644\n> --- a/Documentation/git-cat-file.txt\n> +++ b/Documentation/git-cat-file.txt\n> @@ -9,7 +9,7 @@ git-cat-file - Provide content or type and size information for repository objec\n> SYNOPSIS\n> --------\n> [verse]\n> -'git cat-file' (-t | -s | -e | -p | | --textconv ) \n> +'git cat-file' (-t [--literally]| -s [--literally]| -e | -p | | --textconv ) \n> 'git cat-file' (--batch | --batch-check) < \n>\n> DESCRIPTION\n> @@ -69,6 +69,10 @@ OPTIONS\n> not be combined with any other options or arguments. See the\n> section `BATCH OUTPUT` below for details.\n>\n> +--literally::\n> + Print information of broken/corrupt objects of unknown type without\n> + throwing an error. To be used combined with '-s' or '-t' option.\n\nThe way this is worded, it sounds as if the --literally option itself\nis printing some sort of additional information beyond what -s or -t\noutputs. In fact, it merely allows -s and -t to work with\nbroken/corrupt objects. Perhaps rephrase something like this?\n\n Allow -s or -t to query broken or corrupt objects of\n unknown type.\n\n> OUTPUT\n> ------\n> If '-t' is specified, one of the .\n> --\n> 2.4.0.rc1.249.g9f2ee54\n>\n"},{"id":"259156","messageId":"CAPig+cQ8SqtjRHRp=po0uXHWp3TFDXBsu5W6EUTs=ZWtR-br1A@mail.gmail.com","threadId":"39000","inReplyTo":"1428126289-19239-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v7 4/4] t1006: add tests for git cat-file --literally","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-04-07T20:49:49Z","receivedAt":"2015-04-07T20:49:49Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 4, 2015 at 1:44 AM, Karthik Nayak wrote:\n> Signed-off-by: Karthik Nayak \n> ---\n> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n> index ab36b1e..5b74044 100755\n> --- a/t/t1006-cat-file.sh\n> +++ b/t/t1006-cat-file.sh\n> @@ -47,6 +47,18 @@ $content\"\n> test_cmp expect actual\n> '\n>\n> + test_expect_success \"Type of $type is correct using --literally\" '\n> + echo $type >expect &&\n> + git cat-file -t --literally $sha1 >actual &&\n> + test_cmp expect actual\n> + '\n> +\n> + test_expect_success \"Size of $type is correct using --literally\" '\n> + echo $size >expect &&\n> + git cat-file -s --literally $sha1 >actual &&\n> + test_cmp expect actual\n> + '\n> +\n> test -z \"$content\" ||\n> test_expect_success \"Content of $type is correct\" '\n> maybe_remove_timestamp \"$content\" $no_ts >expect &&\n> @@ -296,4 +308,19 @@ test_expect_success '%(deltabase) reports packed delta bases' '\n> }\n> '\n>\n> +bogus_type=\"bogus\"\n> +bogus_sha1=$(git hash-object -t $bogus_type --literally -w --stdin +\n> +test_expect_success \"Type of broken object is correct\" '\n> + echo $bogus_type >expect &&\n> + git cat-file -t --literally $bogus_sha1 >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success \"Size of broken object is correct\" '\n> + echo \"0\" >expect &&\n\nZero is such a common fallback value when things go wrong that it may\nnot be the best choice for this test. Consequently, it might be better\nto create the bogus object with non-zero length. Take a look at how\n'hello_length' and 'hello_sha1' are computed elsewhere in this script\nfor inspiration.\n\n> + git cat-file -s --literally $bogus_sha1 >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> test_done\n> --\n> 2.4.0.rc1.249.g9f2ee54\n"},{"id":"259181","messageId":"5525689F.5040306@gmail.com","threadId":"39000","inReplyTo":"CAPig+cQ8SqtjRHRp=po0uXHWp3TFDXBsu5W6EUTs=ZWtR-br1A@mail.gmail.com","subject":"Re: [PATCH v7 4/4] t1006: add tests for git cat-file --literally","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-08T17:42:55Z","receivedAt":"2015-04-08T17:42:55Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\nOn 04/08/2015 02:19 AM, Eric Sunshine wrote:\n> On Sat, Apr 4, 2015 at 1:44 AM, Karthik Nayak wrote:\n> > Signed-off-by: Karthik Nayak \n> > ---\n> > diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n> > index ab36b1e..5b74044 100755\n> > --- a/t/t1006-cat-file.sh\n> > +++ b/t/t1006-cat-file.sh\n> > @@ -47,6 +47,18 @@ $content\"\n> > test_cmp expect actual\n> > '\n> >\n> > + test_expect_success \"Type of $type is correct using --literally\" '\n> > + echo $type >expect &&\n> > + git cat-file -t --literally $sha1 >actual &&\n> > + test_cmp expect actual\n> > + '\n> > +\n> > + test_expect_success \"Size of $type is correct using --literally\" '\n> > + echo $size >expect &&\n> > + git cat-file -s --literally $sha1 >actual &&\n> > + test_cmp expect actual\n> > + '\n> > +\n> > test -z \"$content\" ||\n> > test_expect_success \"Content of $type is correct\" '\n> > maybe_remove_timestamp \"$content\" $no_ts >expect &&\n> > @@ -296,4 +308,19 @@ test_expect_success '%(deltabase) reports packed delta bases' '\n> > }\n> > '\n> >\n> > +bogus_type=\"bogus\"\n> > +bogus_sha1=$(git hash-object -t $bogus_type --literally -w --stdin > +\n> > +test_expect_success \"Type of broken object is correct\" '\n> > + echo $bogus_type >expect &&\n> > + git cat-file -t --literally $bogus_sha1 >actual &&\n> > + test_cmp expect actual\n> > +'\n> > +\n> > +test_expect_success \"Size of broken object is correct\" '\n> > + echo \"0\" >expect &&\n>\n> Zero is such a common fallback value when things go wrong that it may\n> not be the best choice for this test. Consequently, it might be better\n> to create the bogus object with non-zero length. Take a look at how\n> 'hello_length' and 'hello_sha1' are computed elsewhere in this script\n> for inspiration.\nThe first part of this patch contains tests which make use of 'hello_length'\nadn 'hello_sha1', but I get what you're saying, will look into it. Thanks.\n>\n> > + git cat-file -s --literally $bogus_sha1 >actual &&\n> > + test_cmp expect actual\n> > +'\n> > +\n> > test_done\n> > --\n> > 2.4.0.rc1.249.g9f2ee54\n"},{"id":"259186","messageId":"CAPig+cTYm-x7oLd+ts6wTXT3Xe_d1Fbw00=0NmrBPCd-+U4_eQ@mail.gmail.com","threadId":"39000","inReplyTo":"5525689F.5040306@gmail.com","subject":"Re: [PATCH v7 4/4] t1006: add tests for git cat-file --literally","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-04-08T20:34:08Z","receivedAt":"2015-04-08T20:34:08Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Apr 8, 2015 at 1:42 PM, karthik nayak wrote:\n> On 04/08/2015 02:19 AM, Eric Sunshine wrote:\n>> On Sat, Apr 4, 2015 at 1:44 AM, Karthik Nayak \n>> wrote:\n>> > Signed-off-by: Karthik Nayak \n>> > ---\n>> > diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n>> > index ab36b1e..5b74044 100755\n>> > --- a/t/t1006-cat-file.sh\n>> > +++ b/t/t1006-cat-file.sh\n>> > @@ -296,4 +308,19 @@ test_expect_success '%(deltabase) reports packed\n>> > delta bases' '\n>> > }\n>> > '\n>> >\n>> > +bogus_type=\"bogus\"\n>> > +bogus_sha1=$(git hash-object -t $bogus_type --literally -w --stdin\n>> > > > +\n>> > +test_expect_success \"Type of broken object is correct\" '\n>> > + echo $bogus_type >expect &&\n>> > + git cat-file -t --literally $bogus_sha1 >actual &&\n>> > + test_cmp expect actual\n>> > +'\n>> > +\n>> > +test_expect_success \"Size of broken object is correct\" '\n>> > + echo \"0\" >expect &&\n>>\n>> Zero is such a common fallback value when things go wrong that it may\n>> not be the best choice for this test. Consequently, it might be better\n>> to create the bogus object with non-zero length. Take a look at how\n>> 'hello_length' and 'hello_sha1' are computed elsewhere in this script\n>> for inspiration.\n>\n> The first part of this patch contains tests which make use of 'hello_length'\n> adn 'hello_sha1', but I get what you're saying, will look into it. Thanks.\n\nJust to be sure we're on the same page, I wasn't suggesting re-using\n'hello_size' and 'hello_sha1', but merely to use that as an example of\nhow to avoid hard-coding the length of your non-zero-length bogus\nobject. So, something like this, perhaps:\n\n bogus_content='bogus'\n bogus_size=$(strlen \"$bogus_content\")\n bogus_sha1=$(echo_without_newline \"$bogus_content\" |\n git hash-object -t $bogus_type --literally -w --stdin)\n ...\n test_expect_success \"Size of broken object is correct\" '\n echo $bogus_size >expect &&\n ...\n"},{"id":"259194","messageId":"346F89A0-8224-453C-AE22-A717D318F43F@gmail.com","threadId":"39000","inReplyTo":"CAPig+cTYm-x7oLd+ts6wTXT3Xe_d1Fbw00=0NmrBPCd-+U4_eQ@mail.gmail.com","subject":"Re: [PATCH v7 4/4] t1006: add tests for git cat-file --literally","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-09T03:24:09Z","receivedAt":"2015-04-09T03:24:09Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\n\nOn April 9, 2015 2:04:08 AM GMT+05:30, Eric Sunshine wrote:\n>On Wed, Apr 8, 2015 at 1:42 PM, karthik nayak \n>wrote:\n>> On 04/08/2015 02:19 AM, Eric Sunshine wrote:\n>>> On Sat, Apr 4, 2015 at 1:44 AM, Karthik Nayak\n>\n>>> wrote:\n>>> > Signed-off-by: Karthik Nayak \n>>> > ---\n>>> > diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n>>> > index ab36b1e..5b74044 100755\n>>> > --- a/t/t1006-cat-file.sh\n>>> > +++ b/t/t1006-cat-file.sh\n>>> > @@ -296,4 +308,19 @@ test_expect_success '%(deltabase) reports\n>packed\n>>> > delta bases' '\n>>> > }\n>>> > '\n>>> >\n>>> > +bogus_type=\"bogus\"\n>>> > +bogus_sha1=$(git hash-object -t $bogus_type --literally -w\n>--stdin\n>>> > >> > +\n>>> > +test_expect_success \"Type of broken object is correct\" '\n>>> > + echo $bogus_type >expect &&\n>>> > + git cat-file -t --literally $bogus_sha1 >actual &&\n>>> > + test_cmp expect actual\n>>> > +'\n>>> > +\n>>> > +test_expect_success \"Size of broken object is correct\" '\n>>> > + echo \"0\" >expect &&\n>>>\n>>> Zero is such a common fallback value when things go wrong that it\n>may\n>>> not be the best choice for this test. Consequently, it might be\n>better\n>>> to create the bogus object with non-zero length. Take a look at how\n>>> 'hello_length' and 'hello_sha1' are computed elsewhere in this\n>script\n>>> for inspiration.\n>>\n>> The first part of this patch contains tests which make use of\n>'hello_length'\n>> adn 'hello_sha1', but I get what you're saying, will look into it.\n>Thanks.\n>\n>Just to be sure we're on the same page, I wasn't suggesting re-using\n>'hello_size' and 'hello_sha1', but merely to use that as an example of\n>how to avoid hard-coding the length of your non-zero-length bogus\n>object. So, something like this, perhaps:\n>\n> bogus_content='bogus'\n> bogus_size=$(strlen \"$bogus_content\")\n> bogus_sha1=$(echo_without_newline \"$bogus_content\" |\n> git hash-object -t $bogus_type --literally -w --stdin)\n> ...\n> test_expect_success \"Size of broken object is correct\" '\n> echo $bogus_size >expect &&\n> ...\nYes, I had done exactly what you suggested here. Thanks \n"}]}