{"thread":{"id":"49334","subject":"[PATCH v1 1/1] Make git_check_attr() a void function","startedAt":"2018-09-12T19:32:09Z","lastAt":"2018-09-12T19:32:09Z","messageCount":1,"participants":["tboegi@web.de"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"358000","messageId":"20180912193202.12896-1-tboegi@web.de","threadId":"49334","inReplyTo":null,"subject":"[PATCH v1 1/1] Make git_check_attr() a void function","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2018-09-12T19:32:02Z","receivedAt":"2018-09-12T19:32:09Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen <tboegi@web.de>\n\ngit_check_attr() returns always 0.\nRemove all the error handling code of the callers, which is never executed.\nChange git_check_attr() to be a void function.\n\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n archive.c              |  3 ++-\n attr.c                 |  8 +++-----\n attr.h                 |  4 ++--\n builtin/check-attr.c   |  3 +--\n builtin/pack-objects.c |  3 +--\n convert.c              | 42 ++++++++++++++++++----------------------\n ll-merge.c             | 16 +++++++--------\n userdiff.c             |  3 +--\n ws.c                   | 44 +++++++++++++++++++-----------------------\n 9 files changed, 57 insertions(+), 69 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 0a07b140fe..c1870105eb 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -110,7 +110,8 @@ static const struct attr_check *get_archive_attrs(struct index_state *istate,\n \tstatic struct attr_check *check;\n \tif (!check)\n \t\tcheck = attr_check_initl(\"export-ignore\", \"export-subst\", NULL);\n-\treturn git_check_attr(istate, path, check) ? NULL : check;\n+\tgit_check_attr(istate, path, check);\n+\treturn check;\n }\n \n static int check_attr_export_ignore(const struct attr_check *check)\ndiff --git a/attr.c b/attr.c\nindex 98e4953f6e..60d284796d 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -1143,9 +1143,9 @@ static void collect_some_attrs(const struct index_state *istate,\n \tfill(path, pathlen, basename_offset, check->stack, check->all_attrs, rem);\n }\n \n-int git_check_attr(const struct index_state *istate,\n-\t\t   const char *path,\n-\t\t   struct attr_check *check)\n+void git_check_attr(const struct index_state *istate,\n+\t\t    const char *path,\n+\t\t    struct attr_check *check)\n {\n \tint i;\n \n@@ -1158,8 +1158,6 @@ int git_check_attr(const struct index_state *istate,\n \t\t\tvalue = ATTR__UNSET;\n \t\tcheck->items[i].value = value;\n \t}\n-\n-\treturn 0;\n }\n \n void git_all_attrs(const struct index_state *istate,\ndiff --git a/attr.h b/attr.h\nindex 2be86db36e..b0378bfe5f 100644\n--- a/attr.h\n+++ b/attr.h\n@@ -63,8 +63,8 @@ void attr_check_free(struct attr_check *check);\n  */\n const char *git_attr_name(const struct git_attr *);\n \n-int git_check_attr(const struct index_state *istate,\n-\t\t   const char *path, struct attr_check *check);\n+void git_check_attr(const struct index_state *istate,\n+\t\t    const char *path, struct attr_check *check);\n \n /*\n  * Retrieve all attributes that apply to the specified path.\ndiff --git a/builtin/check-attr.c b/builtin/check-attr.c\nindex c05573ff9c..30a2f84274 100644\n--- a/builtin/check-attr.c\n+++ b/builtin/check-attr.c\n@@ -65,8 +65,7 @@ static void check_attr(const char *prefix,\n \tif (collect_all) {\n \t\tgit_all_attrs(&the_index, full_path, check);\n \t} else {\n-\t\tif (git_check_attr(&the_index, full_path, check))\n-\t\t\tdie(\"git_check_attr died\");\n+\t\tgit_check_attr(&the_index, full_path, check);\n \t}\n \toutput_attr(check, file);\n \ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex d1144a8f7e..eb71dab5be 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -951,8 +951,7 @@ static int no_try_delta(const char *path)\n \n \tif (!check)\n \t\tcheck = attr_check_initl(\"delta\", NULL);\n-\tif (git_check_attr(&the_index, path, check))\n-\t\treturn 0;\n+\tgit_check_attr(&the_index, path, check);\n \tif (ATTR_FALSE(check->items[0].value))\n \t\treturn 1;\n \treturn 0;\ndiff --git a/convert.c b/convert.c\nindex 6057f1f580..e0848226d2 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1297,6 +1297,7 @@ static void convert_attrs(const struct index_state *istate,\n \t\t\t  struct conv_attrs *ca, const char *path)\n {\n \tstatic struct attr_check *check;\n+\tstruct attr_check_item *ccheck = NULL;\n \n \tif (!check) {\n \t\tcheck = attr_check_initl(\"crlf\", \"ident\", \"filter\",\n@@ -1306,30 +1307,25 @@ static void convert_attrs(const struct index_state *istate,\n \t\tgit_config(read_convert_config, NULL);\n \t}\n \n-\tif (!git_check_attr(istate, path, check)) {\n-\t\tstruct attr_check_item *ccheck = check->items;\n-\t\tca->crlf_action = git_path_check_crlf(ccheck + 4);\n-\t\tif (ca->crlf_action == CRLF_UNDEFINED)\n-\t\t\tca->crlf_action = git_path_check_crlf(ccheck + 0);\n-\t\tca->ident = git_path_check_ident(ccheck + 1);\n-\t\tca->drv = git_path_check_convert(ccheck + 2);\n-\t\tif (ca->crlf_action != CRLF_BINARY) {\n-\t\t\tenum eol eol_attr = git_path_check_eol(ccheck + 3);\n-\t\t\tif (ca->crlf_action == CRLF_AUTO && eol_attr == EOL_LF)\n-\t\t\t\tca->crlf_action = CRLF_AUTO_INPUT;\n-\t\t\telse if (ca->crlf_action == CRLF_AUTO && eol_attr == EOL_CRLF)\n-\t\t\t\tca->crlf_action = CRLF_AUTO_CRLF;\n-\t\t\telse if (eol_attr == EOL_LF)\n-\t\t\t\tca->crlf_action = CRLF_TEXT_INPUT;\n-\t\t\telse if (eol_attr == EOL_CRLF)\n-\t\t\t\tca->crlf_action = CRLF_TEXT_CRLF;\n-\t\t}\n-\t\tca->working_tree_encoding = git_path_check_encoding(ccheck + 5);\n-\t} else {\n-\t\tca->drv = NULL;\n-\t\tca->crlf_action = CRLF_UNDEFINED;\n-\t\tca->ident = 0;\n+\tgit_check_attr(istate, path, check);\n+\tccheck = check->items;\n+\tca->crlf_action = git_path_check_crlf(ccheck + 4);\n+\tif (ca->crlf_action == CRLF_UNDEFINED)\n+\t\tca->crlf_action = git_path_check_crlf(ccheck + 0);\n+\tca->ident = git_path_check_ident(ccheck + 1);\n+\tca->drv = git_path_check_convert(ccheck + 2);\n+\tif (ca->crlf_action != CRLF_BINARY) {\n+\t\tenum eol eol_attr = git_path_check_eol(ccheck + 3);\n+\t\tif (ca->crlf_action == CRLF_AUTO && eol_attr == EOL_LF)\n+\t\t\tca->crlf_action = CRLF_AUTO_INPUT;\n+\t\telse if (ca->crlf_action == CRLF_AUTO && eol_attr == EOL_CRLF)\n+\t\t\tca->crlf_action = CRLF_AUTO_CRLF;\n+\t\telse if (eol_attr == EOL_LF)\n+\t\t\tca->crlf_action = CRLF_TEXT_INPUT;\n+\t\telse if (eol_attr == EOL_CRLF)\n+\t\t\tca->crlf_action = CRLF_TEXT_CRLF;\n \t}\n+\tca->working_tree_encoding = git_path_check_encoding(ccheck + 5);\n \n \t/* Save attr and make a decision for action */\n \tca->attr_action = ca->crlf_action;\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 0e2800f7bb..1936fee9e1 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -371,13 +371,12 @@ int ll_merge(mmbuffer_t *result_buf,\n \tif (!check)\n \t\tcheck = attr_check_initl(\"merge\", \"conflict-marker-size\", NULL);\n \n-\tif (!git_check_attr(&the_index, path, check)) {\n-\t\tll_driver_name = check->items[0].value;\n-\t\tif (check->items[1].value) {\n-\t\t\tmarker_size = atoi(check->items[1].value);\n-\t\t\tif (marker_size <= 0)\n-\t\t\t\tmarker_size = DEFAULT_CONFLICT_MARKER_SIZE;\n-\t\t}\n+\tgit_check_attr(&the_index, path, check);\n+\tll_driver_name = check->items[0].value;\n+\tif (check->items[1].value) {\n+\t\tmarker_size = atoi(check->items[1].value);\n+\t\tif (marker_size <= 0)\n+\t\t\tmarker_size = DEFAULT_CONFLICT_MARKER_SIZE;\n \t}\n \tdriver = find_ll_merge_driver(ll_driver_name);\n \n@@ -398,7 +397,8 @@ int ll_merge_marker_size(const char *path)\n \n \tif (!check)\n \t\tcheck = attr_check_initl(\"conflict-marker-size\", NULL);\n-\tif (!git_check_attr(&the_index, path, check) && check->items[0].value) {\n+\tgit_check_attr(&the_index, path, check);\n+\tif (check->items[0].value) {\n \t\tmarker_size = atoi(check->items[0].value);\n \t\tif (marker_size <= 0)\n \t\t\tmarker_size = DEFAULT_CONFLICT_MARKER_SIZE;\ndiff --git a/userdiff.c b/userdiff.c\nindex f3f4be579c..f565f6731d 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -278,8 +278,7 @@ struct userdiff_driver *userdiff_find_by_path(const char *path)\n \t\tcheck = attr_check_initl(\"diff\", NULL);\n \tif (!path)\n \t\treturn NULL;\n-\tif (git_check_attr(&the_index, path, check))\n-\t\treturn NULL;\n+\tgit_check_attr(&the_index, path, check);\n \n \tif (ATTR_TRUE(check->items[0].value))\n \t\treturn &driver_true;\ndiff --git a/ws.c b/ws.c\nindex 5b67b426e7..a64ab51e09 100644\n--- a/ws.c\n+++ b/ws.c\n@@ -74,35 +74,31 @@ unsigned parse_whitespace_rule(const char *string)\n unsigned whitespace_rule(const char *pathname)\n {\n \tstatic struct attr_check *attr_whitespace_rule;\n+\tconst char *value;\n \n \tif (!attr_whitespace_rule)\n \t\tattr_whitespace_rule = attr_check_initl(\"whitespace\", NULL);\n \n-\tif (!git_check_attr(&the_index, pathname, attr_whitespace_rule)) {\n-\t\tconst char *value;\n-\n-\t\tvalue = attr_whitespace_rule->items[0].value;\n-\t\tif (ATTR_TRUE(value)) {\n-\t\t\t/* true (whitespace) */\n-\t\t\tunsigned all_rule = ws_tab_width(whitespace_rule_cfg);\n-\t\t\tint i;\n-\t\t\tfor (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++)\n-\t\t\t\tif (!whitespace_rule_names[i].loosens_error &&\n-\t\t\t\t    !whitespace_rule_names[i].exclude_default)\n-\t\t\t\t\tall_rule |= whitespace_rule_names[i].rule_bits;\n-\t\t\treturn all_rule;\n-\t\t} else if (ATTR_FALSE(value)) {\n-\t\t\t/* false (-whitespace) */\n-\t\t\treturn ws_tab_width(whitespace_rule_cfg);\n-\t\t} else if (ATTR_UNSET(value)) {\n-\t\t\t/* reset to default (!whitespace) */\n-\t\t\treturn whitespace_rule_cfg;\n-\t\t} else {\n-\t\t\t/* string */\n-\t\t\treturn parse_whitespace_rule(value);\n-\t\t}\n-\t} else {\n+\tgit_check_attr(&the_index, pathname, attr_whitespace_rule);\n+\tvalue = attr_whitespace_rule->items[0].value;\n+\tif (ATTR_TRUE(value)) {\n+\t\t/* true (whitespace) */\n+\t\tunsigned all_rule = ws_tab_width(whitespace_rule_cfg);\n+\t\tint i;\n+\t\tfor (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++)\n+\t\t\tif (!whitespace_rule_names[i].loosens_error &&\n+\t\t\t    !whitespace_rule_names[i].exclude_default)\n+\t\t\t\tall_rule |= whitespace_rule_names[i].rule_bits;\n+\t\treturn all_rule;\n+\t} else if (ATTR_FALSE(value)) {\n+\t\t/* false (-whitespace) */\n+\t\treturn ws_tab_width(whitespace_rule_cfg);\n+\t} else if (ATTR_UNSET(value)) {\n+\t\t/* reset to default (!whitespace) */\n \t\treturn whitespace_rule_cfg;\n+\t} else {\n+\t\t/* string */\n+\t\treturn parse_whitespace_rule(value);\n \t}\n }\n \n-- \n2.18.0\n\n"}]}