Hiroyuki Sano <sh19910711@xxxxxxxxx> writes:
> There were two different ways to check flag values, one way is
> using if-statement, and the other way is using logical expression.
>
> To make sensible, replace if-statements to logical expressions in
> fsck_tree().
The change described by these two paragraphs makes sense to me, but
the "to make sensible" phrasing made me hiccup while reading it.
fsck_tree() uses two different ways to set flag values, many
with a simple if () condition that guards an assignment, and
one with an bitwise-or assignment operator.
Unify them to the latter, as it is shorter and easier to
read when the condition is short and to the point, which all
of them are.
or something?
> When checking "has_dot" and "has_dotdot", use is_dot_or_dotdot()
> instead of strcmp() to avoid hard coding.
I am not sure how this change is an improvement. Besides being
seemingly inefficient by checking name[1] twice (which is not a huge
objection, as a sensible compiler would notice and optimize), the
caller that checks name[1] already hardcodes its knowledge on what
is_dot_or_dotdot() does, e.g. when it returns true, name[0] is never
NUL, and name[1] is NUL only when it saw a dot and not a dotdot, so
the "to avoid hard coding" does not really justify this change.
I further wonder if
...
if (!name[0]) {
has_empty_name = 1;
} else if (name[0] == '.') {
has_dot |= !name[1];
has_dotdot |= name[1] == '.' && !name[2];
has_dotgit |= !strcmp(name + 1, "git");
}
...
may be an improvement (this is not a suggestion--when I say I
wonder, I usually do not know the answer). It defeats the "unify
the two styles" theme of this change, so...
> The is_dot_or_dotdot() is used to check if the string is
> either "." or "..".
> Include the "dir.h" header file to use is_dot_or_dotdot().
>
> Signed-off-by: Hiroyuki Sano <sh19910711@xxxxxxxxx>
> ---
> fsck.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/fsck.c b/fsck.c
> index b3022ad..08f613d 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -6,6 +6,7 @@
> #include "commit.h"
> #include "tag.h"
> #include "fsck.h"
> +#include "dir.h"
>
> static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data)
> {
> @@ -165,18 +166,12 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
>
> sha1 = tree_entry_extract(&desc, &name, &mode);
>
> - if (is_null_sha1(sha1))
> - has_null_sha1 = 1;
> - if (strchr(name, '/'))
> - has_full_path = 1;
> - if (!*name)
> - has_empty_name = 1;
> - if (!strcmp(name, "."))
> - has_dot = 1;
> - if (!strcmp(name, ".."))
> - has_dotdot = 1;
> - if (!strcmp(name, ".git"))
> - has_dotgit = 1;
> + has_null_sha1 |= is_null_sha1(sha1);
> + has_full_path |= !!strchr(name, '/');
> + has_empty_name |= !*name;
> + has_dot |= is_dot_or_dotdot(name) && !name[1];
> + has_dotdot |= is_dot_or_dotdot(name) && name[1];
> + has_dotgit |= !strcmp(name, ".git");
> has_zero_pad |= *(char *)desc.buffer == '0';
> update_tree_entry(&desc);
--
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