Re: [PATCH] setup.c: move statement under condition
Thank you for your replay.
> I have to be honest: this commit message (including the subject) left me
> quite puzzled as to the intent of this patch.
I still only learn English and correctly express my thoughts while somewhat difficult.
Show 5 quoted lines
> If you also have a background story that motivated you to work on this
> patch (for example, if you hit a huge performance bottleneck with some
> tool that fed thousands of absolute paths to Git that needed to be turned
> into paths relative to the worktree's top-level directory), I would
> definitely put that into the commit message, too, if I were you.
I have no such reason. I just saw it and wanted to change it.
Show 6 quoted lines
> Up until recently, we encouraged dropping the curly brackets from
> single-line statements, but apparently that changed. It is now no longer
> clear, and often left to the taste of the contributor. But not always.
> Sometimes we start a beautiful thread discussion the pros and cons of
> curly brackets in the middle of patch review, and drop altogether
> reviewing the actual patch.
I was guided by the rule from the Documentation/CodingGuidelines:
When there are multiple arms to a conditional and some of them
require braces, enclose even a single line block in braces for
consistency.
And other code from setup.c:
from function get_common_dir:
if (!has_common) {
/* several commands */
} else {
free(candidate->work_tree);
}
from function get_common_dir_noenv:
if (file_exists(path.buf)) {
/* several commands */
} else {
strbuf_addstr(sb, gitdir);
}> In short: I think your patch does the right thing, and I hope that you
> find my suggestions to improve the patch useful.
I fixed the patch according to your suggestions.
Signed-off-by: Vadim Petrov <tridronet@yandex.ru>
---
setup.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/setup.c b/setup.c
index 8cc34186c..1a414c256 100644
--- a/setup.c
+++ b/setup.c
@@ -27,26 +27,26 @@ static int abspath_part_inside_repo(char *path)
{
size_t len;
size_t wtlen;
char *path0;
int off;
const char *work_tree = get_git_work_tree();
if (!work_tree)
return -1;
wtlen = strlen(work_tree);
len = strlen(path);
- off = offset_1st_component(path);
- /* check if work tree is already the prefix */
- if (wtlen <= len && !strncmp(path, work_tree, wtlen)) {
+ if (wtlen > len || strncmp(path, work_tree, wtlen))
+ off = offset_1st_component(path);
+ else { /* check if work tree is already the prefix */
if (path[wtlen] == '/') {
memmove(path, path + wtlen + 1, len - wtlen);
return 0;
} else if (path[wtlen - 1] == '/' || path[wtlen] == '\0') {
/* work tree is the root, or the whole path */
memmove(path, path + wtlen, len - wtlen + 1);
return 0;
}
/* work tree might match beginning of a symlink to work tree */
off = wtlen;
}
--
2.15.1.433.g936d1b989