{"thread":{"id":"47891","subject":"[PATCH 1/2] ref-filter: get rid of duplicate code","startedAt":"2018-02-21T06:59:07Z","lastAt":"2018-02-22T05:35:11Z","messageCount":7,"participants":["Olga Telezhnaya","Junio C Hamano","Оля Тележная"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"339793","messageId":"01020161b728823f-daec904e-44ff-45f9-9c89-d447b89c7802-000000@eu-west-1.amazonses.com","threadId":"47891","inReplyTo":null,"subject":"[PATCH 1/2] ref-filter: get rid of duplicate code","fromName":"Olga Telezhnaya","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-02-21T06:59:00Z","receivedAt":"2018-02-21T06:59:07Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"Make one function from 2 duplicate pieces and invoke it twice.\n\nSigned-off-by: Olga Telezhnaia \nMentored-by: Christian Couder \nMentored by: Jeff King \n---\n ref-filter.c | 45 +++++++++++++++++++++------------------------\n 1 file changed, 21 insertions(+), 24 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex f9e25aea7a97e..83ffd84affe52 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1354,15 +1354,31 @@ 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 void get_object(struct ref_array_item *ref, const struct object_id *oid,\n+\t\t int deref, struct object **obj)\n+{\n+\tint eaten;\n+\tunsigned long size;\n+\tvoid *buf = get_obj(oid, obj, &size, &eaten);\n+\tif (!buf)\n+\t\tdie(_(\"missing object %s for %s\"),\n+\t\t oid_to_hex(oid), ref->refname);\n+\tif (!*obj)\n+\t\tdie(_(\"parse_object_buffer failed on %s for %s\"),\n+\t\t oid_to_hex(oid), ref->refname);\n+\n+\tgrab_values(ref->value, deref, *obj, buf, size);\n+\tif (!eaten)\n+\t\tfree(buf);\n+}\n+\n /*\n * Parse the object referred by ref, and grab needed value.\n */\n static void populate_value(struct ref_array_item *ref)\n {\n-\tvoid *buf;\n \tstruct object *obj;\n-\tint eaten, i;\n-\tunsigned long size;\n+\tint i;\n \tconst struct object_id *tagged;\n \n \tref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));\n@@ -1483,17 +1499,7 @@ static void populate_value(struct ref_array_item *ref)\n \treturn;\n \n need_obj:\n-\tbuf = get_obj(&ref->objectname, &obj, &size, &eaten);\n-\tif (!buf)\n-\t\tdie(_(\"missing object %s for %s\"),\n-\t\t oid_to_hex(&ref->objectname), ref->refname);\n-\tif (!obj)\n-\t\tdie(_(\"parse_object_buffer failed on %s for %s\"),\n-\t\t oid_to_hex(&ref->objectname), ref->refname);\n-\n-\tgrab_values(ref->value, 0, obj, buf, size);\n-\tif (!eaten)\n-\t\tfree(buf);\n+\tget_object(ref, &ref->objectname, 0, &obj);\n \n \t/*\n \t * If there is no atom that wants to know about tagged\n@@ -1514,16 +1520,7 @@ static void populate_value(struct ref_array_item *ref)\n \t * is not consistent with what deref_tag() does\n \t * which peels the onion to the core.\n \t */\n-\tbuf = get_obj(tagged, &obj, &size, &eaten);\n-\tif (!buf)\n-\t\tdie(_(\"missing object %s for %s\"),\n-\t\t oid_to_hex(tagged), ref->refname);\n-\tif (!obj)\n-\t\tdie(_(\"parse_object_buffer failed on %s for %s\"),\n-\t\t oid_to_hex(tagged), ref->refname);\n-\tgrab_values(ref->value, 1, obj, buf, size);\n-\tif (!eaten)\n-\t\tfree(buf);\n+\tget_object(ref, tagged, 1, &obj);\n }\n \n /*\n\n--\nhttps://github.com/git/git/pull/460\n"},{"id":"339794","messageId":"01020161b7288324-d478f224-7a37-4341-993f-3c82a9941b7f-000000@eu-west-1.amazonses.com","threadId":"47891","inReplyTo":"01020161b728823f-daec904e-44ff-45f9-9c89-d447b89c7802-000000@eu-west-1.amazonses.com","subject":"[PATCH 2/2] ref-filter: get rid of goto","fromName":"Olga Telezhnaya","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-02-21T06:59:01Z","receivedAt":"2018-02-21T06:59:09Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"Get rid of goto command in ref-filter for better readability.\n\nSigned-off-by: Olga Telezhnaia \nMentored-by: Christian Couder \nMentored by: Jeff King \n---\n ref-filter.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 83ffd84affe52..28df6e21fb996 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1494,11 +1494,11 @@ static void populate_value(struct ref_array_item *ref)\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\tgoto need_obj;\n+\t\t\tbreak;\n \t}\n-\treturn;\n+\tif (used_atom_cnt <= i)\n+\t\treturn;\n \n- need_obj:\n \tget_object(ref, &ref->objectname, 0, &obj);\n \n \t/*\n\n--\nhttps://github.com/git/git/pull/460\n"},{"id":"339795","messageId":"01020161b72bcdff-eb1350fb-12a2-4aaa-abb0-248bf481b2ab-000000@eu-west-1.amazonses.com","threadId":"47891","inReplyTo":"01020161b728823f-daec904e-44ff-45f9-9c89-d447b89c7802-000000@eu-west-1.amazonses.com","subject":"[PATCH v2 1/2] ref-filter: get rid of duplicate code","fromName":"Olga Telezhnaya","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-02-21T07:02:36Z","receivedAt":"2018-02-21T07:02:43Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"Make one function from 2 duplicate pieces and invoke it twice.\n\nSigned-off-by: Olga Telezhnaia \nMentored-by: Christian Couder \nMentored by: Jeff King \n---\n ref-filter.c | 45 +++++++++++++++++++++------------------------\n 1 file changed, 21 insertions(+), 24 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex f9e25aea7a97e..83ffd84affe52 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1354,15 +1354,31 @@ 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 void get_object(struct ref_array_item *ref, const struct object_id *oid,\n+\t\t int deref, struct object **obj)\n+{\n+\tint eaten;\n+\tunsigned long size;\n+\tvoid *buf = get_obj(oid, obj, &size, &eaten);\n+\tif (!buf)\n+\t\tdie(_(\"missing object %s for %s\"),\n+\t\t oid_to_hex(oid), ref->refname);\n+\tif (!*obj)\n+\t\tdie(_(\"parse_object_buffer failed on %s for %s\"),\n+\t\t oid_to_hex(oid), ref->refname);\n+\n+\tgrab_values(ref->value, deref, *obj, buf, size);\n+\tif (!eaten)\n+\t\tfree(buf);\n+}\n+\n /*\n * Parse the object referred by ref, and grab needed value.\n */\n static void populate_value(struct ref_array_item *ref)\n {\n-\tvoid *buf;\n \tstruct object *obj;\n-\tint eaten, i;\n-\tunsigned long size;\n+\tint i;\n \tconst struct object_id *tagged;\n \n \tref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));\n@@ -1483,17 +1499,7 @@ static void populate_value(struct ref_array_item *ref)\n \treturn;\n \n need_obj:\n-\tbuf = get_obj(&ref->objectname, &obj, &size, &eaten);\n-\tif (!buf)\n-\t\tdie(_(\"missing object %s for %s\"),\n-\t\t oid_to_hex(&ref->objectname), ref->refname);\n-\tif (!obj)\n-\t\tdie(_(\"parse_object_buffer failed on %s for %s\"),\n-\t\t oid_to_hex(&ref->objectname), ref->refname);\n-\n-\tgrab_values(ref->value, 0, obj, buf, size);\n-\tif (!eaten)\n-\t\tfree(buf);\n+\tget_object(ref, &ref->objectname, 0, &obj);\n \n \t/*\n \t * If there is no atom that wants to know about tagged\n@@ -1514,16 +1520,7 @@ static void populate_value(struct ref_array_item *ref)\n \t * is not consistent with what deref_tag() does\n \t * which peels the onion to the core.\n \t */\n-\tbuf = get_obj(tagged, &obj, &size, &eaten);\n-\tif (!buf)\n-\t\tdie(_(\"missing object %s for %s\"),\n-\t\t oid_to_hex(tagged), ref->refname);\n-\tif (!obj)\n-\t\tdie(_(\"parse_object_buffer failed on %s for %s\"),\n-\t\t oid_to_hex(tagged), ref->refname);\n-\tgrab_values(ref->value, 1, obj, buf, size);\n-\tif (!eaten)\n-\t\tfree(buf);\n+\tget_object(ref, tagged, 1, &obj);\n }\n \n /*\n\n--\nhttps://github.com/git/git/pull/460\n"},{"id":"339796","messageId":"01020161b72bce58-d3167109-3dd1-40fe-8ad9-727629cd918e-000000@eu-west-1.amazonses.com","threadId":"47891","inReplyTo":"01020161b72bcdff-eb1350fb-12a2-4aaa-abb0-248bf481b2ab-000000@eu-west-1.amazonses.com","subject":"[PATCH v2 2/2] ref-filter: get rid of goto","fromName":"Olga Telezhnaya","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-02-21T07:02:36Z","receivedAt":"2018-02-21T07:02:45Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"Get rid of goto command in ref-filter for better readability.\n\nSigned-off-by: Olga Telezhnaia \nMentored-by: Christian Couder \nMentored by: Jeff King \n---\n ref-filter.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 83ffd84affe52..35359818a1ebb 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1494,11 +1494,11 @@ static void populate_value(struct ref_array_item *ref)\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\tgoto need_obj;\n+\t\t\tbreak;\n \t}\n-\treturn;\n+\tif (i >= used_atom_cnt)\n+\t\treturn;\n \n- need_obj:\n \tget_object(ref, &ref->objectname, 0, &obj);\n \n \t/*\n\n--\nhttps://github.com/git/git/pull/460\n"},{"id":"339812","messageId":"xmqqtvuaz1qq.fsf@gitster-ct.c.googlers.com","threadId":"47891","inReplyTo":"01020161b728823f-daec904e-44ff-45f9-9c89-d447b89c7802-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH 1/2] ref-filter: get rid of duplicate code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-21T17:33:33Z","receivedAt":"2018-02-21T17:33:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Olga Telezhnaya writes:\n\n> Make one function from 2 duplicate pieces and invoke it twice.\n>\n> Signed-off-by: Olga Telezhnaia \n> Mentored-by: Christian Couder \n> Mentored by: Jeff King \n> ---\n> ref-filter.c | 45 +++++++++++++++++++++------------------------\n> 1 file changed, 21 insertions(+), 24 deletions(-)\n\nNice code reduction, removing 2 instances of 11-line section and\nreplacing with a 17-line function ;-)\n\nThis looks related to your earlier changes for \"cat-file --batch\"\nbut I do not recall seeing this refactoring. It seems to be done\ncorrectly.\n\nGood spotting.\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index f9e25aea7a97e..83ffd84affe52 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1354,15 +1354,31 @@ 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 void get_object(struct ref_array_item *ref, const struct object_id *oid,\n> +\t\t int deref, struct object **obj)\n> +{\n> +\tint eaten;\n> +\tunsigned long size;\n> +\tvoid *buf = get_obj(oid, obj, &size, &eaten);\n> +\tif (!buf)\n> +\t\tdie(_(\"missing object %s for %s\"),\n> +\t\t oid_to_hex(oid), ref->refname);\n> +\tif (!*obj)\n> +\t\tdie(_(\"parse_object_buffer failed on %s for %s\"),\n> +\t\t oid_to_hex(oid), ref->refname);\n> +\n> +\tgrab_values(ref->value, deref, *obj, buf, size);\n> +\tif (!eaten)\n> +\t\tfree(buf);\n> +}\n> +\n> /*\n> * Parse the object referred by ref, and grab needed value.\n> */\n> static void populate_value(struct ref_array_item *ref)\n> {\n> -\tvoid *buf;\n> \tstruct object *obj;\n> -\tint eaten, i;\n> -\tunsigned long size;\n> +\tint i;\n> \tconst struct object_id *tagged;\n> \n> \tref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));\n> @@ -1483,17 +1499,7 @@ static void populate_value(struct ref_array_item *ref)\n> \treturn;\n> \n> need_obj:\n> -\tbuf = get_obj(&ref->objectname, &obj, &size, &eaten);\n> -\tif (!buf)\n> -\t\tdie(_(\"missing object %s for %s\"),\n> -\t\t oid_to_hex(&ref->objectname), ref->refname);\n> -\tif (!obj)\n> -\t\tdie(_(\"parse_object_buffer failed on %s for %s\"),\n> -\t\t oid_to_hex(&ref->objectname), ref->refname);\n> -\n> -\tgrab_values(ref->value, 0, obj, buf, size);\n> -\tif (!eaten)\n> -\t\tfree(buf);\n> +\tget_object(ref, &ref->objectname, 0, &obj);\n> \n> \t/*\n> \t * If there is no atom that wants to know about tagged\n> @@ -1514,16 +1520,7 @@ static void populate_value(struct ref_array_item *ref)\n> \t * is not consistent with what deref_tag() does\n> \t * which peels the onion to the core.\n> \t */\n> -\tbuf = get_obj(tagged, &obj, &size, &eaten);\n> -\tif (!buf)\n> -\t\tdie(_(\"missing object %s for %s\"),\n> -\t\t oid_to_hex(tagged), ref->refname);\n> -\tif (!obj)\n> -\t\tdie(_(\"parse_object_buffer failed on %s for %s\"),\n> -\t\t oid_to_hex(tagged), ref->refname);\n> -\tgrab_values(ref->value, 1, obj, buf, size);\n> -\tif (!eaten)\n> -\t\tfree(buf);\n> +\tget_object(ref, tagged, 1, &obj);\n> }\n> \n> /*\n>\n> --\n> https://github.com/git/git/pull/460\n"},{"id":"339813","messageId":"xmqqpo4yz1cw.fsf@gitster-ct.c.googlers.com","threadId":"47891","inReplyTo":"01020161b7288324-d478f224-7a37-4341-993f-3c82a9941b7f-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH 2/2] ref-filter: get rid of goto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-21T17:41:51Z","receivedAt":"2018-02-21T17:41:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Olga Telezhnaya writes:\n\n> Get rid of goto command in ref-filter for better readability.\n>\n> Signed-off-by: Olga Telezhnaia \n> Mentored-by: Christian Couder \n> Mentored by: Jeff King \n> ---\n> ref-filter.c | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n\nIt looks like this is the same change as the bottom-most change on\nyour \"cat-file --batch\" series (and is obviously correct).\n\nI am puzzled by your intention---are you re-organizing and rebooting\nthe series? Either 'Yes' or 'No' is an acceptable answer, and so is\nanything else. I just want to know what you want to happen to the\nmerge conflicts if I queued this while still keeping your \"cat-file\n--batch\" thing I have on 'pu').\n\n>\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 83ffd84affe52..28df6e21fb996 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1494,11 +1494,11 @@ static void populate_value(struct ref_array_item *ref)\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\tgoto need_obj;\n> +\t\t\tbreak;\n> \t}\n> -\treturn;\n> +\tif (used_atom_cnt <= i)\n> +\t\treturn;\n> \n> - need_obj:\n> \tget_object(ref, &ref->objectname, 0, &obj);\n> \n> \t/*\n>\n> --\n> https://github.com/git/git/pull/460\n"},{"id":"339900","messageId":"CAL21Bm=FptUV3xRnYf-iiht8HQF+-uF=cOZyybh7VNCmP=Nxbg@mail.gmail.com","threadId":"47891","inReplyTo":"xmqqpo4yz1cw.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/2] ref-filter: get rid of goto","fromName":"Оля Тележная","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-02-22T05:35:04Z","receivedAt":"2018-02-22T05:35:11Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"2018-02-21 20:41 GMT+03:00 Junio C Hamano :\n> Olga Telezhnaya writes:\n>\n>> Get rid of goto command in ref-filter for better readability.\n>>\n>> Signed-off-by: Olga Telezhnaia \n>> Mentored-by: Christian Couder \n>> Mentored by: Jeff King \n>> ---\n>> ref-filter.c | 6 +++---\n>> 1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> It looks like this is the same change as the bottom-most change on\n> your \"cat-file --batch\" series (and is obviously correct).\n>\n> I am puzzled by your intention---are you re-organizing and rebooting\n> the series? Either 'Yes' or 'No' is an acceptable answer, and so is\n> anything else. I just want to know what you want to happen to the\n> merge conflicts if I queued this while still keeping your \"cat-file\n> --batch\" thing I have on 'pu').\n\nThanks for the question, I needed to mention that.\nWe (with Peff) decided that it's better and easier to remake whole\nprevious patch. Before that, I want to make some refactorings in\nref-filter so after that migrating should go much easier. I want to do\nthat by small separate patches, so this is first in the series. I hope\nit would be both easier to review for you and easier to fix for me.\nSo, if everything is fine, you could merge it to master. I will\nrewrite most parts of previous patch anyway.\n\nThanks!\n\n>\n>>\n>> diff --git a/ref-filter.c b/ref-filter.c\n>> index 83ffd84affe52..28df6e21fb996 100644\n>> --- a/ref-filter.c\n>> +++ b/ref-filter.c\n>> @@ -1494,11 +1494,11 @@ static void populate_value(struct ref_array_item *ref)\n>> for (i = 0; i < used_atom_cnt; i++) {\n>> struct atom_value *v = &ref->value[i];\n>> if (v->s == NULL)\n>> - goto need_obj;\n>> + break;\n>> }\n>> - return;\n>> + if (used_atom_cnt <= i)\n>> + return;\n>>\n>> - need_obj:\n>> get_object(ref, &ref->objectname, 0, &obj);\n>>\n>> /*\n>>\n>> --\n>> https://github.com/git/git/pull/460\n"}]}