Re: [PATCH v3][GSOC] fsck: use bitwise-or assignment operator to set flag
Hiroyuki Sano sh19910...@gmail.com writes:
fsck_tree() has two different ways to set a flag,
which are the followings:
1. Using a if-statement that guards assignment.
2. Using a bitwise-or assignment operator.
Currently, many with the former way,
and one with the latter way.
In this patch, unify them to the latter way,
because it makes the code shorter and easier to read,
and it is brief and to the point.
Two issues:
* In this patch, is redundant.
* it is brief and to the point are equally applicable to both
styles, so that is not a *reason* to choose one over the other.
If a condition were *not* brief and to the point, then a rewrite to
the latter style will make the resulting code worse:
if (a very complex condition
that potentially have to consume a
lot of brain-cycles to understand) {
has_that_condition = 1;
}
is a lot easier to extend than
has_that_condition = (a very complex condition
that potentially have to consume a
lot of brain-cycles to understand);
because it is a lot more likely that we would need to later extend
such a complex condition is more likely than a simple singleton
condition, and we could end up with
if (a very complex condition
that potentially have to consume a
lot of brain-cycles to understand) {
futher computation to check if
the condition really holds
will be added here later
if (does that condition really hold true?)
has_that_condition = 1;
}
which may be harder to express in the latter form.
In other words, it is brief and to the point merely _allows_ these
statements to be expressed in the latter form; it does not say
anything about which is better between the former and the latter.
Signed-off-by: Hiroyuki Sano sh19910...@gmail.com
---
fsck.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/fsck.c b/fsck.c
index b3022ad..abed62b 100644
--- a/fsck.c
+++ b/fsck.c
@@ -165,18 +165,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 |= !strcmp(name, .);
+ has_dotdot |= !strcmp(name, ..);
+ 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3][GSOC] fsck: use bitwise-or assignment operator to set flag
Junio C Hamano gits...@pobox.com writes: In other words, it is brief and to the point merely _allows_ these statements to be expressed in the latter form; it does not say anything about which is better between the former and the latter. In any case, that is a minor point. I'll queue the patch on 'pu', with a tweaked log message. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3][GSOC] fsck: use bitwise-or assignment operator to set flag
Junio and Eric,
Thank you for the reviews and helpful advice.
I should have read more past commit messages before send patch.
Regards,
2014-03-21 3:20 GMT+09:00 Junio C Hamano gits...@pobox.com:
Hiroyuki Sano sh19910...@gmail.com writes:
fsck_tree() has two different ways to set a flag,
which are the followings:
1. Using a if-statement that guards assignment.
2. Using a bitwise-or assignment operator.
Currently, many with the former way,
and one with the latter way.
In this patch, unify them to the latter way,
because it makes the code shorter and easier to read,
and it is brief and to the point.
Two issues:
* In this patch, is redundant.
* it is brief and to the point are equally applicable to both
styles, so that is not a *reason* to choose one over the other.
If a condition were *not* brief and to the point, then a rewrite to
the latter style will make the resulting code worse:
if (a very complex condition
that potentially have to consume a
lot of brain-cycles to understand) {
has_that_condition = 1;
}
is a lot easier to extend than
has_that_condition = (a very complex condition
that potentially have to consume a
lot of brain-cycles to understand);
because it is a lot more likely that we would need to later extend
such a complex condition is more likely than a simple singleton
condition, and we could end up with
if (a very complex condition
that potentially have to consume a
lot of brain-cycles to understand) {
futher computation to check if
the condition really holds
will be added here later
if (does that condition really hold true?)
has_that_condition = 1;
}
which may be harder to express in the latter form.
In other words, it is brief and to the point merely _allows_ these
statements to be expressed in the latter form; it does not say
anything about which is better between the former and the latter.
Signed-off-by: Hiroyuki Sano sh19910...@gmail.com
---
fsck.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/fsck.c b/fsck.c
index b3022ad..abed62b 100644
--- a/fsck.c
+++ b/fsck.c
@@ -165,18 +165,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 |= !strcmp(name, .);
+ has_dotdot |= !strcmp(name, ..);
+ has_dotgit |= !strcmp(name, .git);
has_zero_pad |= *(char *)desc.buffer == '0';
update_tree_entry(desc);
--
Hiroyuki Sano
sh19910...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3][GSOC] fsck: use bitwise-or assignment operator to set flag
fsck_tree() has two different ways to set a flag, which are the followings: 1. Using a if-statement that guards assignment. 2. Using a bitwise-or assignment operator. Currently, many with the former way, and one with the latter way. In this patch, unify them to the latter way, because it makes the code shorter and easier to read, and it is brief and to the point. Signed-off-by: Hiroyuki Sano sh19910...@gmail.com --- fsck.c | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/fsck.c b/fsck.c index b3022ad..abed62b 100644 --- a/fsck.c +++ b/fsck.c @@ -165,18 +165,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 |= !strcmp(name, .); + has_dotdot |= !strcmp(name, ..); + has_dotgit |= !strcmp(name, .git); has_zero_pad |= *(char *)desc.buffer == '0'; update_tree_entry(desc); -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
