Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes: > +static void collect_selected_attrs(const char *path, int num, > + struct git_attr_check *check) > +{ > + struct attr_stack *stk; > + int i, pathlen, rem, dirlen; > + int basename_offset; > + > + pathlen = split_path(path, &dirlen, &basename_offset); > + prepare_attr_stack(path, dirlen); > + if (cannot_trust_maybe_real) { > + for (i = 0; i < git_attr_nr; i++) > + check_all_attr[i].value = ATTR__UNKNOWN; Judging from the fact that (1) the only caller calls this function in this fashion based on the setting of "cannot-trust" bit, (2) this and the other function the only caller calls share the same code in their beginning part, and (3) the body of the if() statement here duplicates the code from collect_all_attrs(), I smell that a much better split is possible. Why isn't this all inside a single function collect_all_attrs()? That single function may no longer be collect_ALL_attrs, so renaming it to collect_attrs() is fine, but then that function may have this if () to initialize all of them to ATTR__UNKNOWN or do the else part we see below, and when organized that way we do not need to have duplicated code (or split_path() helper function), no? > + } else { > + rem = num; > + for (i = 0; i < num; i++) { > + struct git_attr_check *c; > + c = check_all_attr + check[i].attr->attr_nr; > + if (check[i].attr->maybe_real) > + c->value = ATTR__UNKNOWN; > + else { > + c->value = ATTR__UNSET; > + rem--; > + } > + } > + if (!rem) > + return; > + } > + rem = git_attr_nr; > + for (stk = attr_stack; 0 < rem && stk; stk = stk->prev) > + rem = fill(path, pathlen, basename_offset, stk, rem); > +} > + > int git_check_attr(const char *path, int num, struct git_attr_check *check) > { > int i; > > - collect_all_attrs(path); > + if (cannot_trust_maybe_real) > + collect_all_attrs(path); > + else > + collect_selected_attrs(path, num, check); > > for (i = 0; i < num; i++) { > const char *value = check_all_attr[check[i].attr->attr_nr].value; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html