The "ret" was meant to mean "the return value we got from the
callback function", not "the return value we would give our caller".
This rename is a bit misleading in that "cb_bits == -1" does not
mean "full bits set", and it does not tell us much what these "bits"
signify.
They are used to answer this question: which one of the trees in
t[0..n] did the callback function consumed (hence needs their
pointers updated).
So perhaps call it "trees_used" or something?
By the way, our log message usually do not Capitalize the subject
after the "<area>:", i.e. do something like this instead:
Subject: [PATCH 1/3] traverse_trees(): clarify return value of the callback
Thanks.
On 07/19/2013 08:13 PM, Junio C Hamano wrote:
The "ret" was meant to mean "the return value we got from the
callback function", not "the return value we would give our caller".
Thanks for clarifying.
I assumed the "ret" was meant as the return value of that function
as it was the case before e6c111b4c. In other projects I am using
ret as "the return value we would give our caller" as it's such a
convenient name for that if you cannot come up with a better name.
This rename is a bit misleading in that "cb_bits == -1" does not
mean "full bits set", and it does not tell us much what these "bits"
signify.
They are used to answer this question: which one of the trees in
t[0..n] did the callback function consumed (hence needs their
pointers updated).
So perhaps call it "trees_used" or something?
Sounds indeed way better. I'll rename it.
By the way, our log message usually do not Capitalize the subject
after the "<area>:", i.e. do something like this instead:
Subject: [PATCH 1/3] traverse_trees(): clarify return value of the callback
Thanks.
Thanks for pointing out.
As a general question: I was mostly doing micro-optimisations or
the mailmap file, which are rather small fixups, which I think are
ok for beginners. Is there a tasklist for beginners, other than that?
Such as porting shell commands to C or other larger tasks?
I used git://github.com/gitster/git.git as remote/origin. There the todo
branch has the last commit as of 2012/04, so I also found
git://git.kernel.org/pub/scm/git/git.git, where the todo branch seems
more up-to-date, but the TODO file there also seems a little dated to
me.
So is there any up-to-date task list for beginning contributors?
Stefan
The variable name ret sounds like the variable to be returned, but
since e6c111b4 we return error. Hence the variable name is miss leading.
As this variable is used only to extract the bits from the callback of
a tree object, trees_used is a better name.
Also the assignment to 0 was removed at the start of the function as well
after the if(interesting) block. Those were unneeded as that variable
is set to the callback return value any time we enter the if(interesting)
block, so we'd overwrite old values anyway.
Helped-by: Junio C Hamano [off-list ref]
Signed-off-by: Stefan Beller <redacted>
---
tree-walk.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/tree-walk.c b/tree-walk.c
index c366852..5ece8c3 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -324,7 +324,6 @@ static inline int prune_traversal(struct name_entry *e,
int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
{
- int ret = 0;
int error = 0;
struct name_entry *entry = xmalloc(n*sizeof(*entry));
int i;@@ -342,6 +341,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
strbuf_setlen(&base, info->pathlen);
}
for (;;) {
+ int trees_used;
unsigned long mask, dirmask;
const char *first = NULL;
int first_len = 0;@@ -405,15 +405,14 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
if (interesting < 0)
break;
if (interesting) {
- ret = info->fn(n, mask, dirmask, entry, info);
- if (ret < 0) {
- error = ret;
+ trees_used = info->fn(n, mask, dirmask, entry, info);
+ if (trees_used < 0) {
+ error = trees_used;
if (!info->show_all_errors)
break;
}
- mask &= ret;
+ mask &= trees_used;
}
- ret = 0;
for (i = 0; i < n; i++)
if (mask & (1ul << i))
update_extended_entry(tx + i, entry + i);--
1.8.3.3.754.g9c3c367.dirty