{"thread":{"id":"42412","subject":"[PATCH] Do not skip prefix_filename() when there is no prefix","startedAt":"2016-05-22T11:12:03Z","lastAt":"2016-05-22T11:12:03Z","messageCount":1,"participants":["Nguyễn Thái Ngọc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"287201","messageId":"20160522111203.26193-1-pclouds@gmail.com","threadId":"42412","inReplyTo":null,"subject":"[PATCH] Do not skip prefix_filename() when there is no prefix","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-05-22T11:12:03Z","receivedAt":"2016-05-22T11:12:03Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On UNIX, when there is no prefix, prefix_filename() returns the input string\nas-is. Avoiding it only makes sense before 9e5f5d4 (prefix_filename():\nsafely handle the case where pfx_len=0 - 2010-10-17) because the function\nback then could try to dereference a NULL pointer.\n\nOn Windows, even when there is no prefix, prefix_filename() can still do\nsome slash conversion. Arguably it's not the best place for that (too\nsubtle and probably can't cover all paths) but it's how it is.\n\nLet's always call prefix_filename() to make Windows users a tiny bit happier.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n I left out worktree.c on purpose because that call site is handled\n separately [1]. In fact this patch is the result of Junio's comment\n on [1] (its earlier iteration).\n\n [1] http://mid.gmane.org/20160522093356.22389-7-pclouds@gmail.com\n\n builtin/apply.c       | 2 +-\n builtin/hash-object.c | 2 +-\n diff-no-index.c       | 2 +-\n parse-options.c       | 9 ++++++---\n setup.c               | 8 ++++----\n 5 files changed, 13 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 8e4da2e..08f8db3 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -4615,7 +4615,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)\n \t\t\terrs |= apply_patch(0, \"<stdin>\", options);\n \t\t\tread_stdin = 0;\n \t\t\tcontinue;\n-\t\t} else if (0 < prefix_length)\n+\t\t} else\n \t\t\targ = prefix_filename(prefix, prefix_length, arg);\n \n \t\tfd = open(arg, O_RDONLY);\ndiff --git a/builtin/hash-object.c b/builtin/hash-object.c\nindex f7d3567..45be93a 100644\n--- a/builtin/hash-object.c\n+++ b/builtin/hash-object.c\n@@ -110,7 +110,7 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)\n \tif (flags & HASH_WRITE_OBJECT) {\n \t\tprefix = setup_git_directory();\n \t\tprefix_length = prefix ? strlen(prefix) : 0;\n-\t\tif (vpath && prefix)\n+\t\tif (vpath)\n \t\t\tvpath = prefix_filename(prefix, prefix_length, vpath);\n \t}\n \ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 1f8999b..76cc80c 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -268,7 +268,7 @@ void diff_no_index(struct rev_info *revs,\n \t\t\t * path that is \"-\", spell it as \"./-\".\n \t\t\t */\n \t\t\tp = file_from_standard_input;\n-\t\telse if (prefixlen)\n+\t\telse\n \t\t\tp = xstrdup(prefix_filename(prefix, prefixlen, p));\n \t\tpaths[i] = p;\n \t}\ndiff --git a/parse-options.c b/parse-options.c\nindex 47a9192..8b54317 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -37,10 +37,13 @@ static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,\n \n static void fix_filename(const char *prefix, const char **file)\n {\n-\tif (!file || !*file || !prefix || is_absolute_path(*file)\n-\t    || !strcmp(\"-\", *file))\n+\tint prefix_length;\n+\n+\tif (!file || !*file || is_absolute_path(*file) || !strcmp(\"-\", *file))\n \t\treturn;\n-\t*file = xstrdup(prefix_filename(prefix, strlen(prefix), *file));\n+\n+\tprefix_length = prefix ? strlen(prefix) : 0;\n+\t*file = xstrdup(prefix_filename(prefix, prefix_length, *file));\n }\n \n static int opt_command_mode_error(const struct option *opt,\ndiff --git a/setup.c b/setup.c\nindex c86bf5c..291385e 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -141,10 +141,10 @@ int check_filename(const char *prefix, const char *arg)\n \t\tif (arg[2] == '\\0') /* \":/\" is root dir, always exists */\n \t\t\treturn 1;\n \t\tname = arg + 2;\n-\t} else if (prefix)\n-\t\tname = prefix_filename(prefix, strlen(prefix), arg);\n-\telse\n-\t\tname = arg;\n+\t} else {\n+\t\tint prefix_length = prefix ? strlen(prefix) : 0;\n+\t\tname = prefix_filename(prefix, prefix_length, arg);\n+\t}\n \tif (!lstat(name, &st))\n \t\treturn 1; /* file exists */\n \tif (errno == ENOENT || errno == ENOTDIR)\n-- \n2.8.2.524.g6ff3d78\n"}]}