{"thread":{"id":"39183","subject":"[PATCH v9 0/5] cat-file: teach cat-file a '--allow-unkown-type' option","startedAt":"2015-04-29T12:50:31Z","lastAt":"2015-04-30T04:40:11Z","messageCount":17,"participants":["karthik nayak","Karthik Nayak","Junio C Hamano","Phil Hord","Eric Sunshine"],"isPatch":true,"patchVersion":9,"patchTotal":5},"messages":[{"id":"260161","messageId":"5540D397.8020104@gmail.com","threadId":"39183","inReplyTo":null,"subject":"[PATCH v9 0/5] cat-file: teach cat-file a '--allow-unkown-type' option","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-29T12:50:31Z","receivedAt":"2015-04-29T12:50:31Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The last iteration of the patch can be seen :\nhttp://thread.gmane.org/gmane.comp.version-control.git/267213\n\nChanges since last version:\n\nsha1_file:\n* eliminate \"struct strbuf typename = STRBUF_INIT\" in \n\"parse_sha1_header_extended()\"\n* make \"unpack_sha1_header_to_strbuf()\" work automagically. Now it \nunpacks first 32 bytes and checks if the header is unpacked before \nmoving to a strbuf.\n* remove unnecessary if condition for \"strbuf_release()\" .\n\ncat-file:\n* change the option name from \"--literally\" to \"--allow-unkown-type\".\n* make the options mutually exclusive.\n* clean up '-s' option.\n\nt1006:\n* use echo_without_newline() and added quotes.\n"},{"id":"260162","messageId":"1430311933-23180-1-git-send-email-karthik.188@gmail.com","threadId":"39183","inReplyTo":"5540D397.8020104@gmail.com","subject":"[PATCH v9 1/5] sha1_file: support reading from a loose object of unknown type","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-29T12:52:13Z","receivedAt":"2015-04-29T12:52:13Z","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 <gitster@pobox.com>\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nHelped-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\nHepled-by: Jeff King <peff@peff.net>\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n cache.h     |   2 +\n sha1_file.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++-----------\n 2 files changed, 99 insertions(+), 22 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex c47323e..088577d 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_UNKNOWN_OBJECT 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..9a15634 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1564,6 +1564,33 @@ 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, void *buffer,\n+\t\t\t\t\tunsigned long bufsiz, struct strbuf *header)\n+{\n+\tunsigned char *cp;\n+\tint status;\n+\n+\tstatus = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);\n+\n+\t/*\n+\t * Check if entire header is unpacked in the first iteration.\n+\t */\n+\tif (memchr(buffer, '\\0', stream->next_out - (unsigned char *)buffer))\n+\t\treturn 0;\n+\n+\tstrbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);\n+\tdo {\n+\t\tstatus = git_inflate(stream, 0);\n+\t\tstrbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);\n+\t\tif (memchr(buffer, '\\0', stream->next_out - (unsigned char *)buffer))\n+\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 +1641,38 @@ 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+static 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+\tconst char *type_buf = hdr;\n \tunsigned long size;\n+\tint type, type_len = 0;\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 * The type can be of any size but 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\ttype_len++;\n \t}\n-\ttype[i] = 0;\n+\n+\ttype = type_from_string_gently(type_buf, type_len, 1);\n+\tif (oi->typename)\n+\t\tstrbuf_add(oi->typename, type_buf, type_len);\n+\t/*\n+\t * Set type to 0 if its an unknown object and\n+\t * we're obtaining the type using '--allow-unkown-type'\n+\t * option.\n+\t */\n+\tif ((flags & LOOKUP_UNKNOWN_OBJECT) && (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 \n \t/*\n \t * The length must follow immediately, and be in canonical\n@@ -1652,12 +1690,24 @@ 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+\tif (oi->sizep)\n+\t\t*oi->sizep = size;\n \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 +2572,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@@ -2541,7 +2593,7 @@ static int sha1_loose_object_info(const unsigned char *sha1,\n \t * return value implicitly indicates whether the\n \t * object even exists.\n \t */\n-\tif (!oi->typep && !oi->sizep) {\n+\tif (!oi->typep && !oi->typename && !oi->sizep) {\n \t\tstruct stat st;\n \t\tif (stat_sha1_file(sha1, &st) < 0)\n \t\t\treturn -1;\n@@ -2555,17 +2607,24 @@ 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+\tif ((flags & LOOKUP_UNKNOWN_OBJECT)) {\n+\t\tif (unpack_sha1_header_to_strbuf(&stream, map, mapsize, hdr, sizeof(hdr), &hdrbuf) < 0)\n+\t\t\tstatus = error(\"unable to unpack %s header with --allow-unknown-type\",\n+\t\t\t\t       sha1_to_hex(sha1));\n+\t} else if (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+\tif (hdrbuf.len) {\n+\t\tif ((status = parse_sha1_header_extended(hdrbuf.buf, oi, flags)) < 0)\n+\t\t\tstatus = error(\"unable to parse %s header with --allow-unknown-type\",\n+\t\t\t\t       sha1_to_hex(sha1));\n+\t} else if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 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 \tgit_inflate_end(&stream);\n \tmunmap(map, mapsize);\n-\tif (oi->typep)\n+\tif (status && oi->typep)\n \t\t*oi->typep = status;\n+\tstrbuf_release(&hdrbuf);\n \treturn 0;\n }\n \n@@ -2574,6 +2633,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@@ -2586,13 +2646,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@@ -2603,9 +2665,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@@ -2616,6 +2687,10 @@ 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+\tif (oi->typep == &real_type)\n+\t\toi->typep = NULL;\n \n \treturn 0;\n }\n-- \n2.4.0.rc1.250.g565e85b\n"},{"id":"260163","messageId":"1430311953-23240-1-git-send-email-karthik.188@gmail.com","threadId":"39183","inReplyTo":"5540D397.8020104@gmail.com","subject":"[PATCH v9 2/5] cat-file: make the options mutually exclusive","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-29T12:52:33Z","receivedAt":"2015-04-29T12:52:33Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Currently we only parse the options if 2 or 3 arguments are specified.\nUpdate 'struct option options[]' to use OPT_CMDMODE rather than\nOPT_SET_INT to allow only one mutually exclusive option and avoid the\nneed for checking number of arguments. This was written by Junio C Hamano,\ntested by me.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/cat-file.c | 13 +++++--------\n 1 file changed, 5 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex df99df4..53b5376 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -362,12 +362,12 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \n \tconst struct option options[] = {\n \t\tOPT_GROUP(N_(\"<type> can be one of: blob, tree, commit, tag\")),\n-\t\tOPT_SET_INT('t', NULL, &opt, N_(\"show object type\"), 't'),\n-\t\tOPT_SET_INT('s', NULL, &opt, N_(\"show object size\"), 's'),\n-\t\tOPT_SET_INT('e', NULL, &opt,\n+\t\tOPT_CMDMODE('t', NULL, &opt, N_(\"show object type\"), 't'),\n+\t\tOPT_CMDMODE('s', NULL, &opt, N_(\"show object size\"), 's'),\n+\t\tOPT_CMDMODE('e', NULL, &opt,\n \t\t\t    N_(\"exit with zero when there's no error\"), 'e'),\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\tOPT_CMDMODE('p', NULL, &opt, N_(\"pretty-print object's content\"), 'p'),\n+\t\tOPT_CMDMODE(0, \"textconv\", &opt,\n \t\t\t    N_(\"for blob objects, run textconv on object's content\"), 'c'),\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@@ -380,9 +380,6 @@ 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-\t\tusage_with_options(cat_file_usage, options);\n-\n \targc = parse_options(argc, argv, prefix, options, cat_file_usage, 0);\n \n \tif (opt) {\n-- \n2.4.0.rc1.250.g565e85b\n"},{"id":"260164","messageId":"1430312006-23323-1-git-send-email-karthik.188@gmail.com","threadId":"39183","inReplyTo":"5540D397.8020104@gmail.com","subject":"[PATCH v9 3/5] cat-file: teach cat-file a '--allow-unknown-type' option","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-29T12:53:26Z","receivedAt":"2015-04-29T12:53:26Z","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. This is\nbecause these objects are usually of unknown types.\n\nTeach git cat-file a '--allow-unkown-type' 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 '--allow-unkown-type' option.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/cat-file.c | 38 ++++++++++++++++++++++++++------------\n 1 file changed, 26 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 53b5376..299e2e5 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 unkown_type)\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 (unkown_type)\n+\t\tflags |= LOOKUP_UNKNOWN_OBJECT;\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,20 +30,22 @@ 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.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 (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\t\tprintf(\"%lu\\n\", size);\n-\t\t\treturn 0;\n-\t\t}\n-\t\tbreak;\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\tprintf(\"%lu\\n\", size);\n+\t\treturn 0;\n \n \tcase 'e':\n \t\treturn !has_sha1_file(sha1);\n@@ -323,7 +332,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 | <type> | --textconv) <object>\"),\n+\tN_(\"git cat-file (-t [--allow-unkown-type]|-s [--allow-unkown-type]|-e|-p|<type>|--textconv) <object>\"),\n \tN_(\"git cat-file (--batch | --batch-check) < <list-of-objects>\"),\n \tNULL\n };\n@@ -359,6 +368,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 unkown_type = 0;\n \n \tconst struct option options[] = {\n \t\tOPT_GROUP(N_(\"<type> can be one of: blob, tree, commit, tag\")),\n@@ -369,6 +379,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \t\tOPT_CMDMODE('p', NULL, &opt, N_(\"pretty-print object's content\"), 'p'),\n \t\tOPT_CMDMODE(0, \"textconv\", &opt,\n \t\t\t    N_(\"for blob objects, run textconv on object's content\"), 'c'),\n+\t\tOPT_BOOL( 0, \"allow-unkown-type\", &unkown_type,\n+\t\t\t  N_(\"allow -s and -t to work with broken/corrupt objects\")),\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@@ -402,5 +414,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 (unkown_type && opt != 't' && opt != 's')\n+\t\tdie(\"git cat-file --allow-unkown-type: use with -s or -t\");\n+\treturn cat_one_file(opt, exp_type, obj_name, unkown_type);\n }\n-- \n2.4.0.rc1.250.g565e85b\n"},{"id":"260165","messageId":"1430312065-23443-1-git-send-email-karthik.188@gmail.com","threadId":"39183","inReplyTo":"5540D397.8020104@gmail.com","subject":"[PATCH v9 5/5] t1006: add tests for git cat-file --allow-unkown-type","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-29T12:54:25Z","receivedAt":"2015-04-29T12:54:25Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n t/t1006-cat-file.sh | 29 +++++++++++++++++++++++++++++\n 1 file changed, 29 insertions(+)\n\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex ab36b1e..8362019 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 --allow-unkown-type\" '\n+\techo $type >expect &&\n+    git cat-file -t --allow-unkown-type $sha1 >actual &&\n+\ttest_cmp expect actual\n+    '\n+\n+    test_expect_success \"Size of $type is correct using --allow-unkown-type\" '\n+\techo $size >expect &&\n+    git cat-file -s --allow-unkown-type $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,21 @@ test_expect_success '%(deltabase) reports packed delta bases' '\n \t}\n '\n \n+bogus_type=\"bogus\"\n+bogus_content=\"bogus\"\n+bogus_size=$(strlen \"$bogus_content\")\n+bogus_sha1=$(echo_without_newline \"$bogus_content\" | git hash-object -t $bogus_type --literally -w --stdin)\n+\n+test_expect_success \"Type of broken object is correct\" '\n+\techo $bogus_type >expect &&\n+    git cat-file -t --allow-unkown-type $bogus_sha1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success \"Size of broken object is correct\" '\n+\techo $bogus_size >expect &&\n+    git cat-file -s --allow-unkown-type $bogus_sha1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.4.0.rc1.250.g565e85b\n"},{"id":"260166","messageId":"1430312186-23542-1-git-send-email-karthik.188@gmail.com","threadId":"39183","inReplyTo":"5540D397.8020104@gmail.com","subject":"[PATCH v9 4/5] cat-file: add documentation for '--allow-unkown-type' option.","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-29T12:56:26Z","receivedAt":"2015-04-29T12:56:26Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n Documentation/git-cat-file.txt | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex f6a16f4..f6f6064 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 | <type> | --textconv ) <object>\n+'git cat-file' (-t [--allow-unkown-type]| -s [--allow-unkown-type]| -e | -p | <type> | --textconv ) <object>\n 'git cat-file' (--batch | --batch-check) < <list-of-objects>\n \n DESCRIPTION\n@@ -69,6 +69,9 @@ OPTIONS\n \tnot be combined with any other options or arguments.  See the\n \tsection `BATCH OUTPUT` below for details.\n \n+--allow-unkown-type::\n+\tAllow -s or -t to query broken/corrupt objects of unknown type.\n+\n OUTPUT\n ------\n If '-t' is specified, one of the <type>.\n-- \n2.4.0.rc1.250.g565e85b\n"},{"id":"260168","messageId":"xmqqtwvzt2fv.fsf@gitster.dls.corp.google.com","threadId":"39183","inReplyTo":"1430311933-23180-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v9 1/5] sha1_file: support reading from a loose object of unknown type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-29T14:49:08Z","receivedAt":"2015-04-29T14:49:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\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\nThanks.  This mostly looks good modulo a nit.\n\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 980ce6b..9a15634 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -1564,6 +1564,33 @@ 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, void *buffer,\n> +\t\t\t\t\tunsigned long bufsiz, struct strbuf *header)\n\nThis function in this round looks somewhat tricky.\n\n> +{\n> +\tunsigned char *cp;\n> +\tint status;\n> +\n> +\tstatus = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);\n\nHere, we unpack into the caller-supplied buffer, without touching\nthe caller-supplied header strbuf.\n\n> +\t/*\n> +\t * Check if entire header is unpacked in the first iteration.\n> +\t */\n> +\tif (memchr(buffer, '\\0', stream->next_out - (unsigned char *)buffer))\n> +\t\treturn 0;\n\nAnd we return the object header in the buffer without ever touching\nthe header strbuf.  We expect that the caller would know that the\nbuffer it gave us would contain the full object header line.\n\n> +\tstrbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);\n> +\tdo {\n> +\t\tstatus = git_inflate(stream, 0);\n> +\t\tstrbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);\n> +\t\tif (memchr(buffer, '\\0', stream->next_out - (unsigned char *)buffer))\n> +\t\t\treturn 0;\n\nHowever, here, we return the object header in the caller-supplied\nheader strbuf, while using the caller-supplied buffer as a scratch\narea.  We expect that the caller would know that the header strbuf\nis what it has to use to find the object header.\n\nWhich is good in the sense that there is no unnecessary copies, but\nthe caller needs to be careful to do something like:\n\n\tif (!unpack_to_strbuf(... buffer, sizeof(buffer), &header)) {\n    \t\tif (header.len)\n                \tobject_header = header.buf;\n\t\telse\n                \tobject_header = buffer;\n\t} else {\n        \terror(\"cannot unpack the object header\");\n\t}\n\nIt is a good trade-off between complexity and efficiency.  The\ncomplexity is isolated as the function is file-scope-static and it\nis perfectly fine to force the callers to be extra careful.\n\nBut this suggests that the patch to add tests should try at least\ntwo, preferably three, kinds of test input.  A bogus type that needs\na header longer than the caller's fixed buffer, a bogus type whose\nheader would fit within the fixed buffer, and optionally a correct\ntype whose header should always fit within the fixed buffer.\n\n> @@ -1614,27 +1641,38 @@ static void *unpack_sha1_rest(git_zstream *stream, void\n> ...\n> +\t/*\n> +\t * Set type to 0 if its an unknown object and\n> +\t * we're obtaining the type using '--allow-unkown-type'\n> +\t * option.\n> +\t */\n> +\tif ((flags & LOOKUP_UNKNOWN_OBJECT) && (type == -1))\n> +\t\ttype = 0;\n> +\telse if (type == -1)\n> +\t\tdie(\"invalid object type\");\n\nWould \"type == -2\" or any other negative value, if existed, mean\nsomething different?  I do not think so, and hence I would prefer\nthese two checks be done with \"type < 0\" instead.\n\n> @@ -2522,13 +2572,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 ((flags & LOOKUP_UNKNOWN_OBJECT)) {\n> +\t\tif (unpack_sha1_header_to_strbuf(&stream, map, mapsize, hdr, sizeof(hdr), &hdrbuf) < 0)\n> +\t\t\tstatus = error(\"unable to unpack %s header with --allow-unknown-type\",\n> +\t\t\t\t       sha1_to_hex(sha1));\n> +\t} else if (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> +\tif (hdrbuf.len) {\n\nAnd here the caller is being careful, but the way it does is a bit\nsubtle.  Even if it has a codepath that entirely bypasses a call to\nunpack_to_strbuf(), it knows that the other codepath does not touch\nthe hdrbuf strbuf, which means that this block can be reached only\nwhen it called unpack_to_strbuf(), the callee found a long object\nheader, and the unpacking succeeded.  Otherwise, it will make a\ncall to parse_sha1_header_extended() using the fixed hdr[] in the\n\"else if\" clause that corresponds to this \"if\".\n\n> +\t\tif ((status = parse_sha1_header_extended(hdrbuf.buf, oi, flags)) < 0)\n> +\t\t\tstatus = error(\"unable to parse %s header with --allow-unknown-type\",\n> +\t\t\t\t       sha1_to_hex(sha1));\n> +\t} else if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 0)\n>  \t\tstatus = error(\"unable to parse %s header\", sha1_to_hex(sha1));\n\nHowever, I wonder what happens when \"status\" is already set after\nthe call to unpack functions above.  hdrbuf.len may be zero so this\n\"else if\" part would be taken, and it then still try to feed the\nhdr[] to parse_sha1_header_extended(), no?\n\nIn other words, shouldn't the flow be more like this?\n\n\tif (lookup-unknown) {\n        \tif (unpack-to-strbuf)\n\t\t\tstatus = error();\n\t} else if (unpack-to-hdr)\n\t\tstatus = error();\n\n+\tif (status < 0)\n+        \t; /* do not do anything */\n+\telse if (hdrbuf.len)\n-\tif (hdrbuf.len)\n\t\tparse(hdrbuf.buf);\n\telse\n        \tparse(hdr);\n"},{"id":"260169","messageId":"xmqqpp6nt2a7.fsf@gitster.dls.corp.google.com","threadId":"39183","inReplyTo":"1430312006-23323-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v9 3/5] cat-file: teach cat-file a '--allow-unknown-type' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-29T14:52:32Z","receivedAt":"2015-04-29T14:52:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> Currently 'git cat-file' throws an error while trying to\n> print the type or size of a broken/corrupt object. This is\n> because these objects are usually of unknown types.\n>\n> Teach git cat-file a '--allow-unkown-type' option where it prints\n> the type or size of a broken/corrupt object without throwing\n> an error.\n\nDrop \"Currently\" from the description of the problem you are\nsolving.  We know that the problem you have to solve in the code is\ncurrent without being told.  This comment applies to all patches.\n\n> Modify '-t' and '-s' options to call sha1_object_info_extended()\n> directly to support the '--allow-unkown-type' option.\n>\n> Helped-by: Junio C Hamano <gitster@pobox.com>\n> Helped-by: Eric Sunshine <sunshine@sunshineco.com>\n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n>  builtin/cat-file.c | 38 ++++++++++++++++++++++++++------------\n>  1 file changed, 26 insertions(+), 12 deletions(-)\n>\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 53b5376..299e2e5 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 unkown_type)\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 (unkown_type)\n> +\t\tflags |= LOOKUP_UNKNOWN_OBJECT;\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,20 +30,22 @@ 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.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 (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\t\tprintf(\"%lu\\n\", size);\n> -\t\t\treturn 0;\n> -\t\t}\n> -\t\tbreak;\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\tprintf(\"%lu\\n\", size);\n> +\t\treturn 0;\n>  \n>  \tcase 'e':\n>  \t\treturn !has_sha1_file(sha1);\n> @@ -323,7 +332,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 | <type> | --textconv) <object>\"),\n> +\tN_(\"git cat-file (-t [--allow-unkown-type]|-s [--allow-unkown-type]|-e|-p|<type>|--textconv) <object>\"),\n>  \tN_(\"git cat-file (--batch | --batch-check) < <list-of-objects>\"),\n>  \tNULL\n>  };\n> @@ -359,6 +368,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 unkown_type = 0;\n>  \n>  \tconst struct option options[] = {\n>  \t\tOPT_GROUP(N_(\"<type> can be one of: blob, tree, commit, tag\")),\n> @@ -369,6 +379,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n>  \t\tOPT_CMDMODE('p', NULL, &opt, N_(\"pretty-print object's content\"), 'p'),\n>  \t\tOPT_CMDMODE(0, \"textconv\", &opt,\n>  \t\t\t    N_(\"for blob objects, run textconv on object's content\"), 'c'),\n> +\t\tOPT_BOOL( 0, \"allow-unkown-type\", &unkown_type,\n> +\t\t\t  N_(\"allow -s and -t to work with broken/corrupt objects\")),\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> @@ -402,5 +414,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 (unkown_type && opt != 't' && opt != 's')\n> +\t\tdie(\"git cat-file --allow-unkown-type: use with -s or -t\");\n> +\treturn cat_one_file(opt, exp_type, obj_name, unkown_type);\n>  }\n"},{"id":"260170","messageId":"CABURp0qCe3aLtnwueh8Jm6Hm-AN1--Dk1NV4Y=oA5XhMsLh1Aw@mail.gmail.com","threadId":"39183","inReplyTo":"1430312006-23323-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v9 3/5] cat-file: teach cat-file a '--allow-unknown-type' option","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2015-04-29T14:53:13Z","receivedAt":"2015-04-29T14:53:13Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Wed, Apr 29, 2015 at 9:01 AM Karthik Nayak <karthik.188@gmail.com> wrote:\n>\n> Currently 'git cat-file' throws an error while trying to\n> print the type or size of a broken/corrupt object. This is\n> because these objects are usually of unknown types.\n>\n> Teach git cat-file a '--allow-unkown-type' option where it prints\n> the type or size of a broken/corrupt object without throwing\n> an error.\n\nIn this entire series, replace all 'unkown' with 'unknown' in both the\ncommit messages and the code (\"unknown\" is misspelled most of the\ntime).  I notice the switch name itself is misspelled, but also\nvariable names such as 'unkown_type' in this patch.\n\nRespectfully, because I know English is a challenging beast sometimes,\nand spelling is difficult even for many native speakers...\n"},{"id":"260187","messageId":"55411837.40203@gmail.com","threadId":"39183","inReplyTo":"xmqqpp6nt2a7.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v9 3/5] cat-file: teach cat-file a '--allow-unknown-type' option","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-29T17:43:19Z","receivedAt":"2015-04-29T17:43:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\n\nOn 04/29/2015 08:22 PM, Junio C Hamano wrote:\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> Currently 'git cat-file' throws an error while trying to\n>> print the type or size of a broken/corrupt object. This is\n>> because these objects are usually of unknown types.\n>>\n>> Teach git cat-file a '--allow-unkown-type' option where it prints\n>> the type or size of a broken/corrupt object without throwing\n>> an error.\n>\n> Drop \"Currently\" from the description of the problem you are\n> solving.  We know that the problem you have to solve in the code is\n> current without being told.  This comment applies to all patches.\n\nNoted. Thanks!\n>\n>> Modify '-t' and '-s' options to call sha1_object_info_extended()\n>> directly to support the '--allow-unkown-type' option.\n>>\n>> Helped-by: Junio C Hamano <gitster@pobox.com>\n>> Helped-by: Eric Sunshine <sunshine@sunshineco.com>\n>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n>> ---\n>>   builtin/cat-file.c | 38 ++++++++++++++++++++++++++------------\n>>   1 file changed, 26 insertions(+), 12 deletions(-)\n>>\n>> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n>> index 53b5376..299e2e5 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 unkown_type)\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 (unkown_type)\n>> +\t\tflags |= LOOKUP_UNKNOWN_OBJECT;\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,20 +30,22 @@ 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.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 (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\t\tprintf(\"%lu\\n\", size);\n>> -\t\t\treturn 0;\n>> -\t\t}\n>> -\t\tbreak;\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\tprintf(\"%lu\\n\", size);\n>> +\t\treturn 0;\n>>\n>>   \tcase 'e':\n>>   \t\treturn !has_sha1_file(sha1);\n>> @@ -323,7 +332,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 | <type> | --textconv) <object>\"),\n>> +\tN_(\"git cat-file (-t [--allow-unkown-type]|-s [--allow-unkown-type]|-e|-p|<type>|--textconv) <object>\"),\n>>   \tN_(\"git cat-file (--batch | --batch-check) < <list-of-objects>\"),\n>>   \tNULL\n>>   };\n>> @@ -359,6 +368,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 unkown_type = 0;\n>>\n>>   \tconst struct option options[] = {\n>>   \t\tOPT_GROUP(N_(\"<type> can be one of: blob, tree, commit, tag\")),\n>> @@ -369,6 +379,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n>>   \t\tOPT_CMDMODE('p', NULL, &opt, N_(\"pretty-print object's content\"), 'p'),\n>>   \t\tOPT_CMDMODE(0, \"textconv\", &opt,\n>>   \t\t\t    N_(\"for blob objects, run textconv on object's content\"), 'c'),\n>> +\t\tOPT_BOOL( 0, \"allow-unkown-type\", &unkown_type,\n>> +\t\t\t  N_(\"allow -s and -t to work with broken/corrupt objects\")),\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>> @@ -402,5 +414,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 (unkown_type && opt != 't' && opt != 's')\n>> +\t\tdie(\"git cat-file --allow-unkown-type: use with -s or -t\");\n>> +\treturn cat_one_file(opt, exp_type, obj_name, unkown_type);\n>>   }\n"},{"id":"260188","messageId":"55411892.7060205@gmail.com","threadId":"39183","inReplyTo":"CABURp0qCe3aLtnwueh8Jm6Hm-AN1--Dk1NV4Y=oA5XhMsLh1Aw@mail.gmail.com","subject":"Re: [PATCH v9 3/5] cat-file: teach cat-file a '--allow-unknown-type' option","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-29T17:44:50Z","receivedAt":"2015-04-29T17:44:50Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\nOn 04/29/2015 08:23 PM, Phil Hord wrote:\n> On Wed, Apr 29, 2015 at 9:01 AM Karthik Nayak <karthik.188@gmail.com>\n> wrote:\n> >\n> > Currently 'git cat-file' throws an error while trying to\n> > print the type or size of a broken/corrupt object. This is\n> > because these objects are usually of unknown types.\n> >\n> > Teach git cat-file a '--allow-unkown-type' option where it prints\n> > the type or size of a broken/corrupt object without throwing\n> > an error.\n>\n> In this entire series, replace all 'unkown' with 'unknown' in both the\n> commit messages and the code (\"unknown\" is misspelled most of the\n> time).  I notice the switch name itself is misspelled, but also\n> variable names such as 'unkown_type' in this patch.\n>\n> Respectfully, because I know English is a challenging beast sometimes,\n> and spelling is difficult even for many native speakers...\n>\nThanks for that, Yes it does get a bit tricky with spellings, will find \nan editor with spellcheck and avoid nano :D\n"},{"id":"260189","messageId":"554119F6.5010900@gmail.com","threadId":"39183","inReplyTo":"xmqqtwvzt2fv.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v9 1/5] sha1_file: support reading from a loose object of unknown type","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-29T17:50:46Z","receivedAt":"2015-04-29T17:50:46Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\n\nOn 04/29/2015 08:19 PM, Junio C Hamano wrote:\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\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> Thanks.  This mostly looks good modulo a nit.\n\nSorry didn't get what you meant by \"modulo a nit.\".\n>\n>> diff --git a/sha1_file.c b/sha1_file.c\n>> index 980ce6b..9a15634 100644\n>> --- a/sha1_file.c\n>> +++ b/sha1_file.c\n>> @@ -1564,6 +1564,33 @@ 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, void *buffer,\n>> +\t\t\t\t\tunsigned long bufsiz, struct strbuf *header)\n>\n> This function in this round looks somewhat tricky.\n>\n>> +{\n>> +\tunsigned char *cp;\n>> +\tint status;\n>> +\n>> +\tstatus = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);\n>\n> Here, we unpack into the caller-supplied buffer, without touching\n> the caller-supplied header strbuf.\n>\n>> +\t/*\n>> +\t * Check if entire header is unpacked in the first iteration.\n>> +\t */\n>> +\tif (memchr(buffer, '\\0', stream->next_out - (unsigned char *)buffer))\n>> +\t\treturn 0;\n>\n> And we return the object header in the buffer without ever touching\n> the header strbuf.  We expect that the caller would know that the\n> buffer it gave us would contain the full object header line.\n>\n>> +\tstrbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);\n>> +\tdo {\n>> +\t\tstatus = git_inflate(stream, 0);\n>> +\t\tstrbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);\n>> +\t\tif (memchr(buffer, '\\0', stream->next_out - (unsigned char *)buffer))\n>> +\t\t\treturn 0;\n>\n> However, here, we return the object header in the caller-supplied\n> header strbuf, while using the caller-supplied buffer as a scratch\n> area.  We expect that the caller would know that the header strbuf\n> is what it has to use to find the object header.\n>\n> Which is good in the sense that there is no unnecessary copies, but\n> the caller needs to be careful to do something like:\n>\n> \tif (!unpack_to_strbuf(... buffer, sizeof(buffer), &header)) {\n>      \t\tif (header.len)\n>                  \tobject_header = header.buf;\n> \t\telse\n>                  \tobject_header = buffer;\n> \t} else {\n>          \terror(\"cannot unpack the object header\");\n> \t}\n>\n> It is a good trade-off between complexity and efficiency.  The\n> complexity is isolated as the function is file-scope-static and it\n> is perfectly fine to force the callers to be extra careful.\n>\n> But this suggests that the patch to add tests should try at least\n> two, preferably three, kinds of test input.  A bogus type that needs\n> a header longer than the caller's fixed buffer, a bogus type whose\n> header would fit within the fixed buffer, and optionally a correct\n> type whose header should always fit within the fixed buffer.\nYes it is a tradeoff, and it is complex as in the user has to check the \nstrbuf provided to see if its been used. But this like you said I guess \nits a good tradeoff.\nAbout the three tests, My patch checks \"a bogus type whose header would \nfit within the fixed buffer\" and \"correct type whose header should \nalways fit within the fixed buffer\" but will write a test to check \"A \nbogus type that needs a header longer than the caller's fixed buffer\"\n>\n>> @@ -1614,27 +1641,38 @@ static void *unpack_sha1_rest(git_zstream *stream, void\n>> ...\n>> +\t/*\n>> +\t * Set type to 0 if its an unknown object and\n>> +\t * we're obtaining the type using '--allow-unkown-type'\n>> +\t * option.\n>> +\t */\n>> +\tif ((flags & LOOKUP_UNKNOWN_OBJECT) && (type == -1))\n>> +\t\ttype = 0;\n>> +\telse if (type == -1)\n>> +\t\tdie(\"invalid object type\");\n>\n> Would \"type == -2\" or any other negative value, if existed, mean\n> something different?  I do not think so, and hence I would prefer\n> these two checks be done with \"type < 0\" instead.\nThanks will change this.\n>\n>> @@ -2522,13 +2572,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 ((flags & LOOKUP_UNKNOWN_OBJECT)) {\n>> +\t\tif (unpack_sha1_header_to_strbuf(&stream, map, mapsize, hdr, sizeof(hdr), &hdrbuf) < 0)\n>> +\t\t\tstatus = error(\"unable to unpack %s header with --allow-unknown-type\",\n>> +\t\t\t\t       sha1_to_hex(sha1));\n>> +\t} else if (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>> +\tif (hdrbuf.len) {\n>\n> And here the caller is being careful, but the way it does is a bit\n> subtle.  Even if it has a codepath that entirely bypasses a call to\n> unpack_to_strbuf(), it knows that the other codepath does not touch\n> the hdrbuf strbuf, which means that this block can be reached only\n> when it called unpack_to_strbuf(), the callee found a long object\n> header, and the unpacking succeeded.  Otherwise, it will make a\n> call to parse_sha1_header_extended() using the fixed hdr[] in the\n> \"else if\" clause that corresponds to this \"if\".\n>\n>> +\t\tif ((status = parse_sha1_header_extended(hdrbuf.buf, oi, flags)) < 0)\n>> +\t\t\tstatus = error(\"unable to parse %s header with --allow-unknown-type\",\n>> +\t\t\t\t       sha1_to_hex(sha1));\n>> +\t} else if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 0)\n>>   \t\tstatus = error(\"unable to parse %s header\", sha1_to_hex(sha1));\n>\n> However, I wonder what happens when \"status\" is already set after\n> the call to unpack functions above.  hdrbuf.len may be zero so this\n> \"else if\" part would be taken, and it then still try to feed the\n> hdr[] to parse_sha1_header_extended(), no?\n>\n> In other words, shouldn't the flow be more like this?\n>\n> \tif (lookup-unknown) {\n>          \tif (unpack-to-strbuf)\n> \t\t\tstatus = error();\n> \t} else if (unpack-to-hdr)\n> \t\tstatus = error();\n>\n> +\tif (status < 0)\n> +        \t; /* do not do anything */\n> +\telse if (hdrbuf.len)\n> -\tif (hdrbuf.len)\n> \t\tparse(hdrbuf.buf);\n> \telse\n>          \tparse(hdr);\n>\nYes you're right here, you had mentioned it before too, I missed it \nsomehow. Thanks\n"},{"id":"260195","messageId":"xmqq383i6849.fsf@gitster.dls.corp.google.com","threadId":"39183","inReplyTo":"554119F6.5010900@gmail.com","subject":"Re: [PATCH v9 1/5] sha1_file: support reading from a loose object of unknown type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-29T19:35:02Z","receivedAt":"2015-04-29T19:35:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"karthik nayak <karthik.188@gmail.com> writes:\n\n> On 04/29/2015 08:19 PM, Junio C Hamano wrote:\n>> Karthik Nayak <karthik.188@gmail.com> writes:\n>>\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>> Thanks.  This mostly looks good modulo a nit.\n>\n> Sorry didn't get what you meant by \"modulo a nit.\".\n\n\"nit\" as in \"Nit-pick\"; a small imperfection that may need to be\ncorrected (such as the \"what if we saw failure earlier and 'status'\nalready had a value?\" issue).\n\n>> It is a good trade-off between complexity and efficiency.  The\n>> complexity is isolated as the function is file-scope-static and it\n>> is perfectly fine to force the callers to be extra careful.\n>>\n>> But this suggests that the patch to add tests should try at least\n>> two, preferably three, kinds of test input.  A bogus type that needs\n>> a header longer than the caller's fixed buffer, a bogus type whose\n>> header would fit within the fixed buffer, and optionally a correct\n>> type whose header should always fit within the fixed buffer.\n>\n> Yes it is a tradeoff, and it is complex as in the user has to check\n> the strbuf provided to see if its been used. But this like you said I\n> guess its a good tradeoff.\n> About the three tests, My patch checks \"a bogus type whose header\n> would fit within the fixed buffer\" and \"correct type whose header\n> should always fit within the fixed buffer\" but will write a test to\n> check \"A bogus type that needs a header longer than the caller's fixed\n> buffer\"\n\nYup. Please do so; that would make the test coverage more complete.\n"},{"id":"260196","messageId":"xmqqy4la4tdd.fsf@gitster.dls.corp.google.com","threadId":"39183","inReplyTo":"55411892.7060205@gmail.com","subject":"Re: [PATCH v9 3/5] cat-file: teach cat-file a '--allow-unknown-type' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-29T19:38:54Z","receivedAt":"2015-04-29T19:38:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"karthik nayak <karthik.188@gmail.com> writes:\n\n>> Respectfully, because I know English is a challenging beast sometimes,\n>> and spelling is difficult even for many native speakers...\n>>\n> Thanks for that, Yes it does get a bit tricky with spellings, will\n> find an editor with spellcheck and avoid nano :D\n\n;-).\n\nReminds me of a pecurilar habit my old mentor had that I never\npicked up, which is to run \"spell\" on the code, not just comments\nand log messages, to catch misnamed variables.\n"},{"id":"260204","messageId":"CAPig+cREOOEmv=JXu7m8-LoVt0ZErQBht+OyTC=BnXX6FZ96iw@mail.gmail.com","threadId":"39183","inReplyTo":"1430312186-23542-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v9 4/5] cat-file: add documentation for '--allow-unkown-type' option.","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-04-29T21:13:22Z","receivedAt":"2015-04-29T21:13:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Apr 29, 2015 at 8:56 AM, Karthik Nayak <karthik.188@gmail.com> wrote:\n> cat-file: add documentation for '--allow-unkown-type' option.\n\nDrop the end-of-line period.\n\n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n\nIt's not clear why this change is done separately from patch 3/5\n(cat-file: teach cat-file a '--allow-unknown-type' option). Given how\nsmall this patch is and considering how closely related it is to the\nactual introduction of the --allow-unknown-type option, it might make\nsense to fold it into 3/5.\n\n> ---\n> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\n> index f6a16f4..f6f6064 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 | <type> | --textconv ) <object>\n> +'git cat-file' (-t [--allow-unkown-type]| -s [--allow-unkown-type]| -e | -p | <type> | --textconv ) <object>\n>  'git cat-file' (--batch | --batch-check) < <list-of-objects>\n>\n>  DESCRIPTION\n> @@ -69,6 +69,9 @@ OPTIONS\n>         not be combined with any other options or arguments.  See the\n>         section `BATCH OUTPUT` below for details.\n>\n> +--allow-unkown-type::\n> +       Allow -s or -t to query broken/corrupt objects of unknown type.\n> +\n>  OUTPUT\n>  ------\n>  If '-t' is specified, one of the <type>.\n> --\n> 2.4.0.rc1.250.g565e85b\n"},{"id":"260206","messageId":"CAPig+cSXB1fyRGTtuOGsxiBnAhVwS2DydLmVYmq-+47EAa-7-g@mail.gmail.com","threadId":"39183","inReplyTo":"1430312065-23443-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v9 5/5] t1006: add tests for git cat-file --allow-unkown-type","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-04-29T21:16:11Z","receivedAt":"2015-04-29T21:16:11Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Apr 29, 2015 at 8:54 AM, Karthik Nayak <karthik.188@gmail.com> wrote:\n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n> index ab36b1e..8362019 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 --allow-unkown-type\" '\n> +       echo $type >expect &&\n> +    git cat-file -t --allow-unkown-type $sha1 >actual &&\n\nSomehow the indentation got botched here and in the remaining tests\nyou added. This issue is new since v8.\n\n> +       test_cmp expect actual\n> +    '\n> +\n> +    test_expect_success \"Size of $type is correct using --allow-unkown-type\" '\n> +       echo $size >expect &&\n> +    git cat-file -s --allow-unkown-type $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,21 @@ test_expect_success '%(deltabase) reports packed delta bases' '\n>         }\n>  '\n>\n> +bogus_type=\"bogus\"\n> +bogus_content=\"bogus\"\n> +bogus_size=$(strlen \"$bogus_content\")\n> +bogus_sha1=$(echo_without_newline \"$bogus_content\" | 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 --allow-unkown-type $bogus_sha1 >actual &&\n> +       test_cmp expect actual\n> +'\n> +\n> +test_expect_success \"Size of broken object is correct\" '\n> +       echo $bogus_size >expect &&\n> +    git cat-file -s --allow-unkown-type $bogus_sha1 >actual &&\n> +       test_cmp expect actual\n> +'\n> +\n>  test_done\n> --\n> 2.4.0.rc1.250.g565e85b\n"},{"id":"260256","messageId":"5541B22B.6040403@gmail.com","threadId":"39183","inReplyTo":"xmqq383i6849.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v9 1/5] sha1_file: support reading from a loose object of unknown type","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-04-30T04:40:11Z","receivedAt":"2015-04-30T04:40:11Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\nOn 04/30/2015 01:05 AM, Junio C Hamano wrote:\n> karthik nayak <karthik.188@gmail.com> writes:\n>\n> > On 04/29/2015 08:19 PM, Junio C Hamano wrote:\n> >> Karthik Nayak <karthik.188@gmail.com> writes:\n> >>\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> >> Thanks.  This mostly looks good modulo a nit.\n> >\n> > Sorry didn't get what you meant by \"modulo a nit.\".\n>\n> \"nit\" as in \"Nit-pick\"; a small imperfection that may need to be\n> corrected (such as the \"what if we saw failure earlier and 'status'\n> already had a value?\" issue).\nThanks for clearing that out.\n>\n> >> It is a good trade-off between complexity and efficiency.  The\n> >> complexity is isolated as the function is file-scope-static and it\n> >> is perfectly fine to force the callers to be extra careful.\n> >>\n> >> But this suggests that the patch to add tests should try at least\n> >> two, preferably three, kinds of test input.  A bogus type that needs\n> >> a header longer than the caller's fixed buffer, a bogus type whose\n> >> header would fit within the fixed buffer, and optionally a correct\n> >> type whose header should always fit within the fixed buffer.\n> >\n> > Yes it is a tradeoff, and it is complex as in the user has to check\n> > the strbuf provided to see if its been used. But this like you said I\n> > guess its a good tradeoff.\n> > About the three tests, My patch checks \"a bogus type whose header\n> > would fit within the fixed buffer\" and \"correct type whose header\n> > should always fit within the fixed buffer\" but will write a test to\n> > check \"A bogus type that needs a header longer than the caller's fixed\n> > buffer\"\n>\n> Yup. Please do so; that would make the test coverage more complete.\n>\nYup will do :)\n"}]}