{"thread":{"id":"48877","subject":"[PATCH v2 2/4] ref-filter: fill empty fields with empty values","startedAt":"2018-07-13T12:43:59Z","lastAt":"2018-07-17T22:17:56Z","messageCount":7,"participants":["Olga Telezhnaya","Junio C Hamano","Оля Тележная"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"352464","messageId":"0102016493ab53df-b6a5c87b-d9e3-40c0-b125-7843a38103f1-000000@eu-west-1.amazonses.com","threadId":"48877","inReplyTo":"0102016493ab5347-c0429041-6e66-4550-894c-2d500cb2ed8e-000000@eu-west-1.amazonses.com","subject":"[PATCH v2 2/4] ref-filter: fill empty fields with empty values","fromName":"Olga Telezhnaya","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-07-13T12:43:56Z","receivedAt":"2018-07-13T12:43:59Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"Atoms like \"align\" or \"end\" do not have string representation.\nEarlier we had to go and parse whole object with a hope that we\ncould fill their string representations. It's easier to fill them\nwith an empty string before we start to work with whole object.\n\nIt is important to mention that we fill only these atoms that must\ncontain nothing. So, if we could not fill the atom because, for example,\nthe object is missing, we leave it with NULL.\n\nSigned-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>\n---\n ref-filter.c | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 8611c24fd57d1..27733ef013bed 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1497,6 +1497,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\trefname = get_symref(atom, ref);\n \t\telse if (starts_with(name, \"upstream\")) {\n \t\t\tconst char *branch_name;\n+\t\t\tv->s = \"\";\n \t\t\t/* only local branches may have an upstream */\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n \t\t\t\t\t &branch_name))\n@@ -1509,6 +1510,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tcontinue;\n \t\t} else if (atom->u.remote_ref.push) {\n \t\t\tconst char *branch_name;\n+\t\t\tv->s = \"\";\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n \t\t\t\t\t &branch_name))\n \t\t\t\tcontinue;\n@@ -1549,22 +1551,26 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tcontinue;\n \t\t} else if (starts_with(name, \"align\")) {\n \t\t\tv->handler = align_atom_handler;\n+\t\t\tv->s = \"\";\n \t\t\tcontinue;\n \t\t} else if (!strcmp(name, \"end\")) {\n \t\t\tv->handler = end_atom_handler;\n+\t\t\tv->s = \"\";\n \t\t\tcontinue;\n \t\t} else if (starts_with(name, \"if\")) {\n \t\t\tconst char *s;\n-\n+\t\t\tv->s = \"\";\n \t\t\tif (skip_prefix(name, \"if:\", &s))\n \t\t\t\tv->s = xstrdup(s);\n \t\t\tv->handler = if_atom_handler;\n \t\t\tcontinue;\n \t\t} else if (!strcmp(name, \"then\")) {\n \t\t\tv->handler = then_atom_handler;\n+\t\t\tv->s = \"\";\n \t\t\tcontinue;\n \t\t} else if (!strcmp(name, \"else\")) {\n \t\t\tv->handler = else_atom_handler;\n+\t\t\tv->s = \"\";\n \t\t\tcontinue;\n \t\t} else\n \t\t\tcontinue;\n\n--\nhttps://github.com/git/git/pull/520\n"},{"id":"352466","messageId":"0102016493ab5420-51c1c19d-e1ca-43cb-8f9e-36f242f5c972-000000@eu-west-1.amazonses.com","threadId":"48877","inReplyTo":"0102016493ab5347-c0429041-6e66-4550-894c-2d500cb2ed8e-000000@eu-west-1.amazonses.com","subject":"[PATCH v2 3/4] ref-filter: merge get_obj and get_object","fromName":"Olga Telezhnaya","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-07-13T12:43:56Z","receivedAt":"2018-07-13T12:44:00Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"Inline get_obj(): it would be easier to edit the code\nwithout this split.\n\nSigned-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>\n---\n ref-filter.c | 36 +++++++++++-------------------------\n 1 file changed, 11 insertions(+), 25 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 27733ef013bed..f04169f0ea0e3 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -797,24 +797,6 @@ int verify_ref_format(struct ref_format *format)\n \treturn 0;\n }\n \n-/*\n- * Given an object name, read the object data and size, and return a\n- * \"struct object\".  If the object data we are returning is also borrowed\n- * by the \"struct object\" representation, set *eaten as well---it is a\n- * signal from parse_object_buffer to us not to free the buffer.\n- */\n-static void *get_obj(const struct object_id *oid, struct object **obj, unsigned long *sz, int *eaten)\n-{\n-\tenum object_type type;\n-\tvoid *buf = read_object_file(oid, &type, sz);\n-\n-\tif (buf)\n-\t\t*obj = parse_object_buffer(oid, type, *sz, buf, eaten);\n-\telse\n-\t\t*obj = NULL;\n-\treturn buf;\n-}\n-\n static int grab_objectname(const char *name, const struct object_id *oid,\n \t\t\t   struct atom_value *v, struct used_atom *atom)\n {\n@@ -1437,20 +1419,24 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re\n }\n \n static int get_object(struct ref_array_item *ref, const struct object_id *oid,\n-\t\t       int deref, struct object **obj, struct strbuf *err)\n+\t\t      int deref, struct object **obj, struct strbuf *err)\n {\n \tint eaten;\n \tint ret = 0;\n \tunsigned long size;\n-\tvoid *buf = get_obj(oid, obj, &size, &eaten);\n+\tenum object_type type;\n+\tvoid *buf = read_object_file(oid, &type, &size);\n \tif (!buf)\n \t\tret = strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\n \t\t\t\t      oid_to_hex(oid), ref->refname);\n-\telse if (!*obj)\n-\t\tret = strbuf_addf_ret(err, -1, _(\"parse_object_buffer failed on %s for %s\"),\n-\t\t\t\t      oid_to_hex(oid), ref->refname);\n-\telse\n-\t\tgrab_values(ref->value, deref, *obj, buf, size);\n+\telse {\n+\t\t*obj = parse_object_buffer(oid, type, size, buf, &eaten);\n+\t\tif (!*obj)\n+\t\t\tret = strbuf_addf_ret(err, -1, _(\"parse_object_buffer failed on %s for %s\"),\n+\t\t\t\t\t      oid_to_hex(oid), ref->refname);\n+\t\telse\n+\t\t\tgrab_values(ref->value, deref, *obj, buf, size);\n+\t}\n \tif (!eaten)\n \t\tfree(buf);\n \treturn ret;\n\n--\nhttps://github.com/git/git/pull/520\n"},{"id":"352465","messageId":"0102016493ab542d-ea2a3e8c-d6a0-44ca-8fc6-f940fe7e84cd-000000@eu-west-1.amazonses.com","threadId":"48877","inReplyTo":"0102016493ab5347-c0429041-6e66-4550-894c-2d500cb2ed8e-000000@eu-west-1.amazonses.com","subject":"[PATCH v2 4/4] ref-filter: use oid_object_info() to get object","fromName":"Olga Telezhnaya","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-07-13T12:43:56Z","receivedAt":"2018-07-13T12:44:01Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"Use oid_object_info_extended() to get object info instead of\nread_object_file().\nIt will help to handle some requests faster (e.g., we do not need to\nparse whole object if we need to know %(objectsize)).\nIt could also help us to add new atoms such as %(objectsize:disk)\nand %(deltabase).\n\nSigned-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>\n---\n ref-filter.c | 123 ++++++++++++++++++++++++++++++++++++++++++-----------------\n 1 file changed, 89 insertions(+), 34 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex f04169f0ea0e3..112955f006648 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -61,6 +61,17 @@ struct refname_atom {\n \tint lstrip, rstrip;\n };\n \n+static struct expand_data {\n+\tstruct object_id oid;\n+\tenum object_type type;\n+\tunsigned long size;\n+\toff_t disk_size;\n+\tstruct object_id delta_base_oid;\n+\tvoid *content;\n+\n+\tstruct object_info info;\n+} oi, oi_deref;\n+\n /*\n  * An atom is a valid field atom listed below, possibly prefixed with\n  * a \"*\" to denote deref_tag().\n@@ -202,6 +213,30 @@ static int remote_ref_atom_parser(const struct ref_format *format, struct used_a\n \treturn 0;\n }\n \n+static int objecttype_atom_parser(const struct ref_format *format, struct used_atom *atom,\n+\t\t\t\t  const char *arg, struct strbuf *err)\n+{\n+\tif (arg)\n+\t\treturn strbuf_addf_ret(err, -1, _(\"%%(objecttype) does not take arguments\"));\n+\tif (*atom->name == '*')\n+\t\toi_deref.info.typep = &oi_deref.type;\n+\telse\n+\t\toi.info.typep = &oi.type;\n+\treturn 0;\n+}\n+\n+static int objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom,\n+\t\t\t\t  const char *arg, struct strbuf *err)\n+{\n+\tif (arg)\n+\t\treturn strbuf_addf_ret(err, -1, _(\"%%(objectsize) does not take arguments\"));\n+\tif (*atom->name == '*')\n+\t\toi_deref.info.sizep = &oi_deref.size;\n+\telse\n+\t\toi.info.sizep = &oi.size;\n+\treturn 0;\n+}\n+\n static int body_atom_parser(const struct ref_format *format, struct used_atom *atom,\n \t\t\t    const char *arg, struct strbuf *err)\n {\n@@ -388,8 +423,8 @@ static struct {\n \t\t      const char *arg, struct strbuf *err);\n } valid_atom[] = {\n \t{ \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n-\t{ \"objecttype\", SOURCE_OTHER },\n-\t{ \"objectsize\", SOURCE_OTHER, FIELD_ULONG },\n+\t{ \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },\n+\t{ \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },\n \t{ \"objectname\", SOURCE_OTHER, FIELD_STR, objectname_atom_parser },\n \t{ \"tree\", SOURCE_OBJ },\n \t{ \"parent\", SOURCE_OBJ },\n@@ -502,6 +537,12 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \tused_atom[at].name = xmemdupz(atom, ep - atom);\n \tused_atom[at].type = valid_atom[i].cmp_type;\n \tused_atom[at].source = valid_atom[i].source;\n+\tif (used_atom[at].source == SOURCE_OBJ) {\n+\t\tif (*atom == '*')\n+\t\t\toi_deref.info.contentp = &oi_deref.content;\n+\t\telse\n+\t\t\toi.info.contentp = &oi.content;\n+\t}\n \tif (arg) {\n \t\targ = used_atom[at].name + (arg - atom) + 1;\n \t\tif (!*arg) {\n@@ -817,7 +858,7 @@ static int grab_objectname(const char *name, const struct object_id *oid,\n }\n \n /* See grab_values */\n-static void grab_common_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n+static void grab_common_values(struct atom_value *val, int deref, struct expand_data *oi)\n {\n \tint i;\n \n@@ -829,13 +870,13 @@ static void grab_common_values(struct atom_value *val, int deref, struct object\n \t\tif (deref)\n \t\t\tname++;\n \t\tif (!strcmp(name, \"objecttype\"))\n-\t\t\tv->s = type_name(obj->type);\n+\t\t\tv->s = type_name(oi->type);\n \t\telse if (!strcmp(name, \"objectsize\")) {\n-\t\t\tv->value = sz;\n-\t\t\tv->s = xstrfmt(\"%lu\", sz);\n+\t\t\tv->value = oi->size;\n+\t\t\tv->s = xstrfmt(\"%lu\", oi->size);\n \t\t}\n \t\telse if (deref)\n-\t\t\tgrab_objectname(name, &obj->oid, v, &used_atom[i]);\n+\t\t\tgrab_objectname(name, &oi->oid, v, &used_atom[i]);\n \t}\n }\n \n@@ -1194,7 +1235,6 @@ static void fill_missing_values(struct atom_value *val)\n  */\n static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n {\n-\tgrab_common_values(val, deref, obj, buf, sz);\n \tswitch (obj->type) {\n \tcase OBJ_TAG:\n \t\tgrab_tag_values(val, deref, obj, buf, sz);\n@@ -1418,28 +1458,36 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re\n \treturn show_ref(&atom->u.refname, ref->refname);\n }\n \n-static int get_object(struct ref_array_item *ref, const struct object_id *oid,\n-\t\t      int deref, struct object **obj, struct strbuf *err)\n+static int get_object(struct ref_array_item *ref, int deref, struct object **obj,\n+\t\t      struct expand_data *oi, struct strbuf *err)\n {\n-\tint eaten;\n-\tint ret = 0;\n-\tunsigned long size;\n-\tenum object_type type;\n-\tvoid *buf = read_object_file(oid, &type, &size);\n-\tif (!buf)\n-\t\tret = strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\n-\t\t\t\t      oid_to_hex(oid), ref->refname);\n-\telse {\n-\t\t*obj = parse_object_buffer(oid, type, size, buf, &eaten);\n-\t\tif (!*obj)\n-\t\t\tret = strbuf_addf_ret(err, -1, _(\"parse_object_buffer failed on %s for %s\"),\n-\t\t\t\t\t      oid_to_hex(oid), ref->refname);\n-\t\telse\n-\t\t\tgrab_values(ref->value, deref, *obj, buf, size);\n+\t/* parse_object_buffer() will set eaten to 0 if free() will be needed */\n+\tint eaten = 1;\n+\tif (oi->info.contentp) {\n+\t\t/* We need to know that to use parse_object_buffer properly */\n+\t\toi->info.sizep = &oi->size;\n+\t\toi->info.typep = &oi->type;\n \t}\n+\tif (oid_object_info_extended(the_repository, &oi->oid, &oi->info,\n+\t\t\t\t     OBJECT_INFO_LOOKUP_REPLACE))\n+\t\treturn strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\n+\t\t\t\t       oid_to_hex(&oi->oid), ref->refname);\n+\n+\tif (oi->info.contentp) {\n+\t\t*obj = parse_object_buffer(&oi->oid, oi->type, oi->size, oi->content, &eaten);\n+\t\tif (!obj) {\n+\t\t\tif (!eaten)\n+\t\t\t\tfree(oi->content);\n+\t\t\treturn strbuf_addf_ret(err, -1, _(\"parse_object_buffer failed on %s for %s\"),\n+\t\t\t\t\t       oid_to_hex(&oi->oid), ref->refname);\n+\t\t}\n+\t\tgrab_values(ref->value, deref, *obj, oi->content, oi->size);\n+\t}\n+\n+\tgrab_common_values(ref->value, deref, oi);\n \tif (!eaten)\n-\t\tfree(buf);\n-\treturn ret;\n+\t\tfree(oi->content);\n+\treturn 0;\n }\n \n /*\n@@ -1449,7 +1497,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n {\n \tstruct object *obj;\n \tint i;\n-\tconst struct object_id *tagged;\n+\tstruct object_info empty = OBJECT_INFO_INIT;\n \n \tref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));\n \n@@ -1569,13 +1617,20 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tstruct atom_value *v = &ref->value[i];\n-\t\tif (v->s == NULL)\n-\t\t\tbreak;\n+\t\tif (v->s == NULL && used_atom[i].source == SOURCE_NONE)\n+\t\t\treturn strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\n+\t\t\t\t\t       oid_to_hex(&ref->objectname), ref->refname);\n \t}\n-\tif (used_atom_cnt <= i)\n+\n+\tif (need_tagged)\n+\t\toi.info.contentp = &oi.content;\n+\tif (!memcmp(&oi.info, &empty, sizeof(empty)) &&\n+\t    !memcmp(&oi_deref.info, &empty, sizeof(empty)))\n \t\treturn 0;\n \n-\tif (get_object(ref, &ref->objectname, 0, &obj, err))\n+\n+\toi.oid = ref->objectname;\n+\tif (get_object(ref, 0, &obj, &oi, err))\n \t\treturn -1;\n \n \t/*\n@@ -1589,7 +1644,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t * If it is a tag object, see if we use a value that derefs\n \t * the object, and if we do grab the object it refers to.\n \t */\n-\ttagged = &((struct tag *)obj)->tagged->oid;\n+\toi_deref.oid = ((struct tag *)obj)->tagged->oid;\n \n \t/*\n \t * NEEDSWORK: This derefs tag only once, which\n@@ -1597,7 +1652,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t * is not consistent with what deref_tag() does\n \t * which peels the onion to the core.\n \t */\n-\treturn get_object(ref, tagged, 1, &obj, err);\n+\treturn get_object(ref, 1, &obj, &oi_deref, err);\n }\n \n /*\n\n--\nhttps://github.com/git/git/pull/520\n"},{"id":"352467","messageId":"0102016493ab5347-c0429041-6e66-4550-894c-2d500cb2ed8e-000000@eu-west-1.amazonses.com","threadId":"48877","inReplyTo":"010201647e19c0f0-68cd728f-c86a-4be2-be6b-fd7e19e930fa-000000@eu-west-1.amazonses.com","subject":"[PATCH v2 1/4] ref-filter: add info_source to valid_atom","fromName":"Olga Telezhnaya","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-07-13T12:43:56Z","receivedAt":"2018-07-13T12:44:03Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"Add the source of object data to prevent parsing of unneeded data.\nThe goal is to improve performance by avoiding calling expensive\nfunctions when we don't need the information they provide\nor when we could get it by using a cheaper function.\n\nIt is stored in valid_atoms because it depends on the atoms we are\ninterested in.\n\nSigned-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>\n---\n ref-filter.c | 82 +++++++++++++++++++++++++++++++-----------------------------\n 1 file changed, 43 insertions(+), 39 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex fa3685d91f046..8611c24fd57d1 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -41,6 +41,7 @@ void setup_ref_filter_porcelain_msg(void)\n \n typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;\n typedef enum { COMPARE_EQUAL, COMPARE_UNEQUAL, COMPARE_NONE } cmp_status;\n+typedef enum { SOURCE_NONE = 0, SOURCE_OBJ, SOURCE_OTHER } info_source;\n \n struct align {\n \talign_type position;\n@@ -73,6 +74,7 @@ struct refname_atom {\n static struct used_atom {\n \tconst char *name;\n \tcmp_type type;\n+\tinfo_source source;\n \tunion {\n \t\tchar color[COLOR_MAXLEN];\n \t\tstruct align align;\n@@ -380,49 +382,50 @@ static int head_atom_parser(const struct ref_format *format, struct used_atom *a\n \n static struct {\n \tconst char *name;\n+\tinfo_source source;\n \tcmp_type cmp_type;\n \tint (*parser)(const struct ref_format *format, struct used_atom *atom,\n \t\t      const char *arg, struct strbuf *err);\n } valid_atom[] = {\n-\t{ \"refname\" , FIELD_STR, refname_atom_parser },\n-\t{ \"objecttype\" },\n-\t{ \"objectsize\", FIELD_ULONG },\n-\t{ \"objectname\", FIELD_STR, objectname_atom_parser },\n-\t{ \"tree\" },\n-\t{ \"parent\" },\n-\t{ \"numparent\", FIELD_ULONG },\n-\t{ \"object\" },\n-\t{ \"type\" },\n-\t{ \"tag\" },\n-\t{ \"author\" },\n-\t{ \"authorname\" },\n-\t{ \"authoremail\" },\n-\t{ \"authordate\", FIELD_TIME },\n-\t{ \"committer\" },\n-\t{ \"committername\" },\n-\t{ \"committeremail\" },\n-\t{ \"committerdate\", FIELD_TIME },\n-\t{ \"tagger\" },\n-\t{ \"taggername\" },\n-\t{ \"taggeremail\" },\n-\t{ \"taggerdate\", FIELD_TIME },\n-\t{ \"creator\" },\n-\t{ \"creatordate\", FIELD_TIME },\n-\t{ \"subject\", FIELD_STR, subject_atom_parser },\n-\t{ \"body\", FIELD_STR, body_atom_parser },\n-\t{ \"trailers\", FIELD_STR, trailers_atom_parser },\n-\t{ \"contents\", FIELD_STR, contents_atom_parser },\n-\t{ \"upstream\", FIELD_STR, remote_ref_atom_parser },\n-\t{ \"push\", FIELD_STR, remote_ref_atom_parser },\n-\t{ \"symref\", FIELD_STR, refname_atom_parser },\n-\t{ \"flag\" },\n-\t{ \"HEAD\", FIELD_STR, head_atom_parser },\n-\t{ \"color\", FIELD_STR, color_atom_parser },\n-\t{ \"align\", FIELD_STR, align_atom_parser },\n-\t{ \"end\" },\n-\t{ \"if\", FIELD_STR, if_atom_parser },\n-\t{ \"then\" },\n-\t{ \"else\" },\n+\t{ \"refname\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n+\t{ \"objecttype\", SOURCE_OTHER },\n+\t{ \"objectsize\", SOURCE_OTHER, FIELD_ULONG },\n+\t{ \"objectname\", SOURCE_OTHER, FIELD_STR, objectname_atom_parser },\n+\t{ \"tree\", SOURCE_OBJ },\n+\t{ \"parent\", SOURCE_OBJ },\n+\t{ \"numparent\", SOURCE_OBJ, FIELD_ULONG },\n+\t{ \"object\", SOURCE_OBJ },\n+\t{ \"type\", SOURCE_OBJ },\n+\t{ \"tag\", SOURCE_OBJ },\n+\t{ \"author\", SOURCE_OBJ },\n+\t{ \"authorname\", SOURCE_OBJ },\n+\t{ \"authoremail\", SOURCE_OBJ },\n+\t{ \"authordate\", SOURCE_OBJ, FIELD_TIME },\n+\t{ \"committer\", SOURCE_OBJ },\n+\t{ \"committername\", SOURCE_OBJ },\n+\t{ \"committeremail\", SOURCE_OBJ },\n+\t{ \"committerdate\", SOURCE_OBJ, FIELD_TIME },\n+\t{ \"tagger\", SOURCE_OBJ },\n+\t{ \"taggername\", SOURCE_OBJ },\n+\t{ \"taggeremail\", SOURCE_OBJ },\n+\t{ \"taggerdate\", SOURCE_OBJ, FIELD_TIME },\n+\t{ \"creator\", SOURCE_OBJ },\n+\t{ \"creatordate\", SOURCE_OBJ, FIELD_TIME },\n+\t{ \"subject\", SOURCE_OBJ, FIELD_STR, subject_atom_parser },\n+\t{ \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n+\t{ \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n+\t{ \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n+\t{ \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n+\t{ \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n+\t{ \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n+\t{ \"flag\", SOURCE_NONE },\n+\t{ \"HEAD\", SOURCE_NONE, FIELD_STR, head_atom_parser },\n+\t{ \"color\", SOURCE_NONE, FIELD_STR, color_atom_parser },\n+\t{ \"align\", SOURCE_NONE, FIELD_STR, align_atom_parser },\n+\t{ \"end\", SOURCE_NONE },\n+\t{ \"if\", SOURCE_NONE, FIELD_STR, if_atom_parser },\n+\t{ \"then\", SOURCE_NONE },\n+\t{ \"else\", SOURCE_NONE },\n };\n \n #define REF_FORMATTING_STATE_INIT  { 0, NULL }\n@@ -498,6 +501,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \tREALLOC_ARRAY(used_atom, used_atom_cnt);\n \tused_atom[at].name = xmemdupz(atom, ep - atom);\n \tused_atom[at].type = valid_atom[i].cmp_type;\n+\tused_atom[at].source = valid_atom[i].source;\n \tif (arg) {\n \t\targ = used_atom[at].name + (arg - atom) + 1;\n \t\tif (!*arg) {\n\n--\nhttps://github.com/git/git/pull/520\n"},{"id":"352677","messageId":"xmqqr2k2sy02.fsf@gitster-ct.c.googlers.com","threadId":"48877","inReplyTo":"0102016493ab542d-ea2a3e8c-d6a0-44ca-8fc6-f940fe7e84cd-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH v2 4/4] ref-filter: use oid_object_info() to get object","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-16T20:53:49Z","receivedAt":"2018-07-16T20:53:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:\n\n> -static int get_object(struct ref_array_item *ref, const struct object_id *oid,\n> -\t\t      int deref, struct object **obj, struct strbuf *err)\n> +static int get_object(struct ref_array_item *ref, int deref, struct object **obj,\n> +\t\t      struct expand_data *oi, struct strbuf *err)\n>  {\n> -\tint eaten;\n> -\tint ret = 0;\n> -\tunsigned long size;\n> -\tenum object_type type;\n> -\tvoid *buf = read_object_file(oid, &type, &size);\n> -\tif (!buf)\n> -\t\tret = strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\n> -\t\t\t\t      oid_to_hex(oid), ref->refname);\n> -\telse {\n> -\t\t*obj = parse_object_buffer(oid, type, size, buf, &eaten);\n> -\t\tif (!*obj)\n> -\t\t\tret = strbuf_addf_ret(err, -1, _(\"parse_object_buffer failed on %s for %s\"),\n> -\t\t\t\t\t      oid_to_hex(oid), ref->refname);\n> -\t\telse\n> -\t\t\tgrab_values(ref->value, deref, *obj, buf, size);\n> +\t/* parse_object_buffer() will set eaten to 0 if free() will be needed */\n> +\tint eaten = 1;\n\nHmph, doesn't this belong to the previous step?  In other words,\nisn't the result of applying 1-3/4 has a bug that can leave eaten\nuninitialized (and base decision to free(buf) later on it), and\nisn't this change a fix for it?\n\n> +\tif (oi->info.contentp) {\n> +\t\t/* We need to know that to use parse_object_buffer properly */\n> +\t\toi->info.sizep = &oi->size;\n> +\t\toi->info.typep = &oi->type;\n>  \t}\n> +\tif (oid_object_info_extended(the_repository, &oi->oid, &oi->info,\n> +\t\t\t\t     OBJECT_INFO_LOOKUP_REPLACE))\n> +\t\treturn strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\n> +\t\t\t\t       oid_to_hex(&oi->oid), ref->refname);\n> +\n> +\tif (oi->info.contentp) {\n> +\t\t*obj = parse_object_buffer(&oi->oid, oi->type, oi->size, oi->content, &eaten);\n> +\t\tif (!obj) {\n> +\t\t\tif (!eaten)\n> +\t\t\t\tfree(oi->content);\n> +\t\t\treturn strbuf_addf_ret(err, -1, _(\"parse_object_buffer failed on %s for %s\"),\n> +\t\t\t\t\t       oid_to_hex(&oi->oid), ref->refname);\n> +\t\t}\n> +\t\tgrab_values(ref->value, deref, *obj, oi->content, oi->size);\n> +\t}\n> +\n> +\tgrab_common_values(ref->value, deref, oi);\n>  \tif (!eaten)\n> -\t\tfree(buf);\n> -\treturn ret;\n> +\t\tfree(oi->content);\n> +\treturn 0;\n>  }\n\n"},{"id":"352744","messageId":"CAL21BmnDqv9yib5Vqh4q7zj1hd_bmhO+xnDVJBi-zM0Cqtsevw@mail.gmail.com","threadId":"48877","inReplyTo":"xmqqr2k2sy02.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 4/4] ref-filter: use oid_object_info() to get object","fromName":"Оля Тележная","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-07-17T07:44:33Z","receivedAt":"2018-07-17T07:44:37Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"2018-07-16 23:53 GMT+03:00 Junio C Hamano <gitster@pobox.com>:\n> Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:\n>\n>> -static int get_object(struct ref_array_item *ref, const struct object_id *oid,\n>> -                   int deref, struct object **obj, struct strbuf *err)\n>> +static int get_object(struct ref_array_item *ref, int deref, struct object **obj,\n>> +                   struct expand_data *oi, struct strbuf *err)\n>>  {\n>> -     int eaten;\n>> -     int ret = 0;\n>> -     unsigned long size;\n>> -     enum object_type type;\n>> -     void *buf = read_object_file(oid, &type, &size);\n>> -     if (!buf)\n>> -             ret = strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\n>> -                                   oid_to_hex(oid), ref->refname);\n>> -     else {\n>> -             *obj = parse_object_buffer(oid, type, size, buf, &eaten);\n>> -             if (!*obj)\n>> -                     ret = strbuf_addf_ret(err, -1, _(\"parse_object_buffer failed on %s for %s\"),\n>> -                                           oid_to_hex(oid), ref->refname);\n>> -             else\n>> -                     grab_values(ref->value, deref, *obj, buf, size);\n>> +     /* parse_object_buffer() will set eaten to 0 if free() will be needed */\n>> +     int eaten = 1;\n>\n> Hmph, doesn't this belong to the previous step?  In other words,\n> isn't the result of applying 1-3/4 has a bug that can leave eaten\n> uninitialized (and base decision to free(buf) later on it), and\n> isn't this change a fix for it?\n\nOh. I was thinking that it was new bug created by me. Now I see that\npreviously we had the same problem. I guess it's better to make\nseparate commit to fix it. Hope I can do it in this patch (I don't\nwant to create separate patch because of so minor update).\nThank you! I will fix it soon.\n\n>\n>> +     if (oi->info.contentp) {\n>> +             /* We need to know that to use parse_object_buffer properly */\n>> +             oi->info.sizep = &oi->size;\n>> +             oi->info.typep = &oi->type;\n>>       }\n>> +     if (oid_object_info_extended(the_repository, &oi->oid, &oi->info,\n>> +                                  OBJECT_INFO_LOOKUP_REPLACE))\n>> +             return strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\n>> +                                    oid_to_hex(&oi->oid), ref->refname);\n>> +\n>> +     if (oi->info.contentp) {\n>> +             *obj = parse_object_buffer(&oi->oid, oi->type, oi->size, oi->content, &eaten);\n>> +             if (!obj) {\n>> +                     if (!eaten)\n>> +                             free(oi->content);\n>> +                     return strbuf_addf_ret(err, -1, _(\"parse_object_buffer failed on %s for %s\"),\n>> +                                            oid_to_hex(&oi->oid), ref->refname);\n>> +             }\n>> +             grab_values(ref->value, deref, *obj, oi->content, oi->size);\n>> +     }\n>> +\n>> +     grab_common_values(ref->value, deref, oi);\n>>       if (!eaten)\n>> -             free(buf);\n>> -     return ret;\n>> +             free(oi->content);\n>> +     return 0;\n>>  }\n>\n"},{"id":"352840","messageId":"xmqqtvoxld68.fsf@gitster-ct.c.googlers.com","threadId":"48877","inReplyTo":"CAL21BmnDqv9yib5Vqh4q7zj1hd_bmhO+xnDVJBi-zM0Cqtsevw@mail.gmail.com","subject":"Re: [PATCH v2 4/4] ref-filter: use oid_object_info() to get object","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-17T22:17:51Z","receivedAt":"2018-07-17T22:17:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Оля Тележная  <olyatelezhnaya@gmail.com> writes:\n\n>> Hmph, doesn't this belong to the previous step?  In other words,\n>> isn't the result of applying 1-3/4 has a bug that can leave eaten\n>> uninitialized (and base decision to free(buf) later on it), and\n>> isn't this change a fix for it?\n>\n> Oh. I was thinking that it was new bug created by me. Now I see that\n> previously we had the same problem.\n\nThe original said something like:\n\n\tint eaten;\n\tvoid *buf = get_obj(..., &eaten);\n\t...\n\tif (!eaten)\n\t\tfree(buf);\n\nand get_obj() left eaten untouched when it returned NULL.  As a\nrandom uninitialized cruft in eaten that happened to be \"true\" would\njust cause free(NULL) on many archs, there was no practical problem\nin such a code, but it is undefined behaviour nevertheless.\n\nAnd the previous step made it a bit more alarming ;-)\n\n"}]}