{"thread":{"id":"19352","subject":"[PATCH 1/3] dir.c: clean up handling of 'path' parameter in read_directory_recursive()","startedAt":"2009-05-14T20:42:47Z","lastAt":"2009-05-19T13:31:48Z","messageCount":13,"participants":["Linus Torvalds","Johannes Schindelin","Aaron Cohen","Junio C Hamano","Jens Kilian","John Koleszar"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"113960","messageId":"alpine.LFD.2.01.0905141341470.3343@localhost.localdomain","threadId":"19352","inReplyTo":null,"subject":"[PATCH 1/3] dir.c: clean up handling of 'path' parameter in read_directory_recursive()","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-14T20:42:47Z","receivedAt":"2009-05-14T20:42:47Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nFrom: Linus Torvalds \nDate: Thu, 14 May 2009 13:05:03 -0700\n\nRight now we pass two different pathnames ('path' and 'base') down to\nread_directory_recursive(), and the only real reason for that is that we\nwant to allow an empty 'base' parameter, but when we do so, we need the\npathname to \"opendir()\" to be \".\" rather than the empty string.\n\nAnd rather than handle that confusion in the caller, we can just fix\nread_directory_recursive() to handle the case of an empty path itself,\nby just passing opendir() a \".\" ourselves if the path is empty.\n\nThis would allow us to then drop one of the pathnames entirely from the\ncalling convention, but rather than do that, we'll start separating them\nout as a \"filesystem pathname\" (the one we use for filesystem accesses)\nand a \"git internal base name\" (which is the name that we use for git\ninternally).\n\nThat will eventually allow us to do things like handle different\nencodings (eg the filesystem pathnames might be Latin1, while git itself\nwould use UTF-8 for filename information).\n\nSigned-off-by: Linus Torvalds \n---\n\nThis is a truly trivial diff, but it's independent from the other changes \nI have, and simplifies the next ones, so I've made it a patch of its own.\n\n dir.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 6aae09a..0e6b752 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -576,7 +576,7 @@ static int get_dtype(struct dirent *de, const char *path)\n */\n static int read_directory_recursive(struct dir_struct *dir, const char *path, const char *base, int baselen, int check_only, const struct path_simplify *simplify)\n {\n-\tDIR *fdir = opendir(path);\n+\tDIR *fdir = opendir(*path ? path : \".\");\n \tint contents = 0;\n \n \tif (fdir) {\n-- \n1.6.3.1.11.g97114\n"},{"id":"113961","messageId":"alpine.LFD.2.01.0905141342520.3343@localhost.localdomain","threadId":"19352","inReplyTo":"alpine.LFD.2.01.0905141341470.3343@localhost.localdomain","subject":"[PATCH 2/3] Add 'fill_directory()' helper function for directory traversal","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-14T20:46:39Z","receivedAt":"2009-05-14T20:46:39Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nFrom: Linus Torvalds \nDate: Thu, 14 May 2009 13:22:36 -0700\n\nMost of the users of \"read_directory()\" actually want a much simpler\ninterface than the whole complex (but rather powerful) one.\n\nIn fact 'git add' had already largely abstracted out the core interface\nissues into a private \"fill_directory()\" function that was largely\napplicable almost as-is to a number of callers. Yes, 'git add' wants to\ndo some extra work of its own, specific to the add semantics, but we can\neasily split that out, and use the core as a generic function.\n\nThis function does exactly that, and now that much simplified\n'fill_directory()' function can be shared with a number of callers,\nwhile also ensuring that the rather more complex calling conventions of\nread_directory() are used by fewer call-sites.\n\nSigned-off-by: Linus Torvalds \n---\n\nAs you can see from the diffstat, this removes more lines than it adds, \nand generally simplifies some calling conventions.\n\nThe return value from \"fill_directory()\" makes little sense for any other \nuser than the builtin-add.c case, and I'm not really proud of it, but it \nbasically allows everybody to share the same general infrastructure.\n\nNote: this depends on the previous one, in that we now use the empty \nstring (\"\") as the \"path\" argument, and now almost all users of \nread_directory() will pass in the same thing as both \"path\" and \"base\". \n\nThat will eventually change, though, if we want to have different \nencodings.\n\n builtin-add.c | 45 ++++++++++++++-------------------------------\n builtin-clean.c | 12 +-----------\n builtin-ls-files.c | 7 +------\n dir.c | 21 +++++++++++++++++++++\n dir.h | 1 +\n wt-status.c | 2 +-\n 6 files changed, 39 insertions(+), 49 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex cb67d2c..ba25893 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -95,35 +95,6 @@ static void treat_gitlinks(const char **pathspec)\n \t}\n }\n \n-static void fill_directory(struct dir_struct *dir, const char **pathspec,\n-\t\tint ignored_too)\n-{\n-\tconst char *path, *base;\n-\tint baselen;\n-\n-\t/* Set up the default git porcelain excludes */\n-\tmemset(dir, 0, sizeof(*dir));\n-\tif (!ignored_too) {\n-\t\tdir->flags |= DIR_COLLECT_IGNORED;\n-\t\tsetup_standard_excludes(dir);\n-\t}\n-\n-\t/*\n-\t * Calculate common prefix for the pathspec, and\n-\t * use that to optimize the directory walk\n-\t */\n-\tbaselen = common_prefix(pathspec);\n-\tpath = \".\";\n-\tbase = \"\";\n-\tif (baselen)\n-\t\tpath = base = xmemdupz(*pathspec, baselen);\n-\n-\t/* Read the directory and prune it */\n-\tread_directory(dir, path, base, baselen, pathspec);\n-\tif (pathspec)\n-\t\tprune_directory(dir, pathspec, baselen);\n-}\n-\n static void refresh(int verbose, const char **pathspec)\n {\n \tchar *seen;\n@@ -290,9 +261,21 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tdie(\"index file corrupt\");\n \ttreat_gitlinks(pathspec);\n \n-\tif (add_new_files)\n+\tif (add_new_files) {\n+\t\tint baselen;\n+\n+\t\t/* Set up the default git porcelain excludes */\n+\t\tmemset(&dir, 0, sizeof(dir));\n+\t\tif (!ignored_too) {\n+\t\t\tdir.flags |= DIR_COLLECT_IGNORED;\n+\t\t\tsetup_standard_excludes(&dir);\n+\t\t}\n+\n \t\t/* This picks up the paths that are not tracked */\n-\t\tfill_directory(&dir, pathspec, ignored_too);\n+\t\tbaselen = fill_directory(&dir, pathspec);\n+\t\tif (pathspec)\n+\t\t\tprune_directory(&dir, pathspec, baselen);\n+\t}\n \n \tif (refresh_only) {\n \t\trefresh(verbose, pathspec);\ndiff --git a/builtin-clean.c b/builtin-clean.c\nindex c5ad33d..febd10f 100644\n--- a/builtin-clean.c\n+++ b/builtin-clean.c\n@@ -33,7 +33,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tint ignored_only = 0, baselen = 0, config_set = 0, errors = 0;\n \tstruct strbuf directory = STRBUF_INIT;\n \tstruct dir_struct dir;\n-\tconst char *path, *base;\n \tstatic const char **pathspec;\n \tstruct strbuf buf = STRBUF_INIT;\n \tconst char *qname;\n@@ -77,16 +76,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tpathspec = get_pathspec(prefix, argv);\n \tread_cache();\n \n-\t/*\n-\t * Calculate common prefix for the pathspec, and\n-\t * use that to optimize the directory walk\n-\t */\n-\tbaselen = common_prefix(pathspec);\n-\tpath = \".\";\n-\tbase = \"\";\n-\tif (baselen)\n-\t\tpath = base = xmemdupz(*pathspec, baselen);\n-\tread_directory(&dir, path, base, baselen, pathspec);\n+\tfill_directory(&dir, pathspec);\n \n \tif (pathspec)\n \t\tseen = xmalloc(argc > 0 ? argc : 1);\ndiff --git a/builtin-ls-files.c b/builtin-ls-files.c\nindex da2daf4..a011a42 100644\n--- a/builtin-ls-files.c\n+++ b/builtin-ls-files.c\n@@ -161,12 +161,7 @@ static void show_files(struct dir_struct *dir, const char *prefix)\n \n \t/* For cached/deleted files we don't need to even do the readdir */\n \tif (show_others || show_killed) {\n-\t\tconst char *path = \".\", *base = \"\";\n-\t\tint baselen = prefix_len;\n-\n-\t\tif (baselen)\n-\t\t\tpath = base = prefix;\n-\t\tread_directory(dir, path, base, baselen, pathspec);\n+\t\tfill_directory(dir, pathspec);\n \t\tif (show_others)\n \t\t\tshow_other_files(dir);\n \t\tif (show_killed)\ndiff --git a/dir.c b/dir.c\nindex 0e6b752..c667d38 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -9,6 +9,27 @@\n #include \"dir.h\"\n #include \"refs.h\"\n \n+int fill_directory(struct dir_struct *dir, const char **pathspec)\n+{\n+\tconst char *path, *base;\n+\tint baselen;\n+\n+\t/*\n+\t * Calculate common prefix for the pathspec, and\n+\t * use that to optimize the directory walk\n+\t */\n+\tbaselen = common_prefix(pathspec);\n+\tpath = \"\";\n+\tbase = \"\";\n+\n+\tif (baselen)\n+\t\tpath = base = xmemdupz(*pathspec, baselen);\n+\n+\t/* Read the directory and prune it */\n+\tread_directory(dir, path, base, baselen, pathspec);\n+\treturn baselen;\n+}\n+\n struct path_simplify {\n \tint len;\n \tconst char *path;\ndiff --git a/dir.h b/dir.h\nindex 541286a..9f7c3ba 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -68,6 +68,7 @@ extern int common_prefix(const char **pathspec);\n #define MATCHED_EXACTLY 3\n extern int match_pathspec(const char **pathspec, const char *name, int namelen, int prefix, char *seen);\n \n+extern int fill_directory(struct dir_struct *dir, const char **pathspec);\n extern int read_directory(struct dir_struct *, const char *path, const char *base, int baselen, const char **pathspec);\n \n extern int excluded(struct dir_struct *, const char *, int *);\ndiff --git a/wt-status.c b/wt-status.c\nindex 1b6df45..24a6abf 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -255,7 +255,7 @@ static void wt_status_print_untracked(struct wt_status *s)\n \t\t\tDIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;\n \tsetup_standard_excludes(&dir);\n \n-\tread_directory(&dir, \".\", \"\", 0, NULL);\n+\tfill_directory(&dir, NULL);\n \tfor(i = 0; i < dir.nr; i++) {\n \t\tstruct dir_entry *ent = dir.entries[i];\n \t\tif (!cache_name_is_other(ent->name, ent->len))\n-- \n1.6.3.1.11.g97114\n"},{"id":"113962","messageId":"alpine.LFD.2.01.0905141346440.3343@localhost.localdomain","threadId":"19352","inReplyTo":"alpine.LFD.2.01.0905141342520.3343@localhost.localdomain","subject":"[PATCH 3/3] read_directory(): infrastructure for pathname character set conversion","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-14T20:54:41Z","receivedAt":"2009-05-14T20:54:41Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nFrom: Linus Torvalds \nDate: Thu, 14 May 2009 13:31:59 -0700\n\nThis is some early first infrastructure to make it much easier for\nread_directory() to recursively traversing a filesystem directory\nstructure, while at the same time doing a character set or naming\nconversion during traversal.\n\nIn particular, this allows:\n\n - the filesystem path component separator to be set to something\n different than the normal UNIX '/' character.\n\n Nobody may care (even windows tends to handle '/' well), but it also\n happens to be a good way to test the feature, and this patch makes\n the filesystem separator be \"//\" (_two_ slashes) just to verify that\n the code correctly keeps the \"filesystem representation\" from the\n \"git internal filename representation\".\n\n - We could - for example - read filesystems that have pathnames in\n a Latin1 encoding, and use this to convert such filesystem character\n set details into a git format (where UTF-8 would be the default, the\n same way we default to UTF-8 in commit messages without actually\n _forcing_ it)\n\n - On OS X, we can make the read_directory() conversion convert the\n native (and odd/crazy) UTF-8 NFD representation into the more normal\n cross-platform NFC representation.\n\nBut please note that this is just preliminary, and not only doesn't\nactually have any explicit character set convrsion code, it still lacks\nsome other infrastructure.\n\nSigned-off-by: Linus Torvalds \n---\n\nOk, so this adds many more lines than I just removed, but at least several \nof them are comments about what the difference between 'path' and 'base' \nis, and it all works.\n\nThe use of \"//\" as the filesystem path component separator may be odd, but \nit's a useful hack: from 'strace', you can now see how different \noperations now use the \"filesystem pathname\" and others use the \"native \ngit pathname\", because one will have two slashes between components and \nthe other will not.\n\nSo you can literally -visually- see the places that aren't converted, and \nthat use the git internal paths for filesystem operations. Example strace \noutput:\n\n ...\n open(\"compat//fnmatch//\", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 6\n getdents(6, /* 4 entries */, 32768) = 112\n open(\"compat/fnmatch/.gitignore\", O_RDONLY) = -1 ENOENT (No such file or directory)\n ...\n\niow, here we see how the directory traversal itself uses the \"filesystem \npathname\", but then the ignore-file handling does not.\n\nNow imagine if one of them needs to do some UTF-8 -> EUC-JP translation or \nsomething like that, rather than having the (purely visual) extra '/'.\n\nSo it's currently just a cheezy hack, but it was useful for testing, and I \nthink this series is worth thinking seriously about. The two first patches \nwere plain cleanups, and this one isn't _that_ complex, but adds some \npotentially interesting infrastructure, even if it's not complete yet.\n\n dir.c | 80 ++++++++++++++++++++++++++++++++++++++++++++-----------\n unpack-trees.c | 4 ++-\n 2 files changed, 67 insertions(+), 17 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex c667d38..ae1ae61 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -22,6 +22,7 @@ int fill_directory(struct dir_struct *dir, const char **pathspec)\n \tpath = \"\";\n \tbase = \"\";\n \n+\t/* FIXME! Filesystem character set vs git internal character set! */\n \tif (baselen)\n \t\tpath = base = xmemdupz(*pathspec, baselen);\n \n@@ -586,6 +587,21 @@ static int get_dtype(struct dirent *de, const char *path)\n \treturn dtype;\n }\n \n+/* No actual conversion yet */\n+static int convert_path_to_git(const char *path, int plen, char *result)\n+{\n+\tmemcpy(result, path, plen+1);\n+\treturn plen;\n+}\n+\n+/*\n+ * For testing!\n+ *\n+ * On Windows, maybe we want FS_PATH_SEP being \"\\\\\"?\n+ */\n+#define FS_PATH_SEP \"//\"\n+#define FS_PATH_SEP_LEN 2\n+\n /*\n * Read a directory tree. We currently ignore anything but\n * directories, regular files and symlinks. That's because git\n@@ -594,37 +610,62 @@ static int get_dtype(struct dirent *de, const char *path)\n *\n * Also, we ignore the name \".git\" (even if it is not a directory).\n * That likely will not change.\n+ *\n+ * 'path' is the filesystem name of directory, in the filesystem\n+ * namespace, while 'base' is the internal git path (converted\n+ * into the standard git namespace).\n */\n-static int read_directory_recursive(struct dir_struct *dir, const char *path, const char *base, int baselen, int check_only, const struct path_simplify *simplify)\n+static int read_directory_recursive(struct dir_struct *dir,\n+\tconst char *path,\n+\tconst char *base, int baselen,\n+\tint check_only, const struct path_simplify *simplify)\n {\n \tDIR *fdir = opendir(*path ? path : \".\");\n \tint contents = 0;\n \n \tif (fdir) {\n+\t\tint pathlen = strlen(path);\n \t\tstruct dirent *de;\n-\t\tchar fullname[PATH_MAX + 1];\n-\t\tmemcpy(fullname, base, baselen);\n+\t\tchar newpath[PATH_MAX + 1];\n+\t\tchar newbase[PATH_MAX + 1];\n+\n+\t\tmemcpy(newpath, path, pathlen);\n+\t\tmemcpy(newbase, base, baselen);\n \n \t\twhile ((de = readdir(fdir)) != NULL) {\n-\t\t\tint len, dtype;\n+\t\t\tchar converted[256];\n+\t\t\tint len, dtype, nlen;\n \t\t\tint exclude;\n \n \t\t\tif (is_dot_or_dotdot(de->d_name) ||\n \t\t\t !strcmp(de->d_name, \".git\"))\n \t\t\t\tcontinue;\n+\n \t\t\tlen = strlen(de->d_name);\n+\n \t\t\t/* Ignore overly long pathnames! */\n-\t\t\tif (len + baselen + 8 > sizeof(fullname))\n+\t\t\tif (len + pathlen + 8 > sizeof(newpath))\n \t\t\t\tcontinue;\n-\t\t\tmemcpy(fullname + baselen, de->d_name, len+1);\n-\t\t\tif (simplify_away(fullname, baselen + len, simplify))\n+\t\t\tmemcpy(newpath + pathlen, de->d_name, len+1);\n+\n+\t\t\tnlen = convert_path_to_git(de->d_name, len, converted);\n+\t\t\tif (nlen + baselen + 8 > sizeof(newbase))\n \t\t\t\tcontinue;\n+\t\t\tmemcpy(newbase + baselen, converted, nlen+1);\n \n+\t\t\tlen = pathlen + len;\n+\t\t\tnlen = baselen + nlen;\n+\n+\t\t\t/* We simplify by the git internal pathname (newbase) */\n+\t\t\tif (simplify_away(newbase, nlen, simplify))\n+\t\t\t\tcontinue;\n+\n+\t\t\t/* Similarly, exclude rules work on the git pathname */\n \t\t\tdtype = DTYPE(de);\n-\t\t\texclude = excluded(dir, fullname, &dtype);\n+\t\t\texclude = excluded(dir, newbase, &dtype);\n \t\t\tif (exclude && (dir->flags & DIR_COLLECT_IGNORED)\n-\t\t\t && in_pathspec(fullname, baselen + len, simplify))\n-\t\t\t\tdir_add_ignored(dir, fullname, baselen + len);\n+\t\t\t && in_pathspec(newbase, nlen, simplify))\n+\t\t\t\tdir_add_ignored(dir, newbase, nlen);\n \n \t\t\t/*\n \t\t\t * Excluded? If we don't explicitly want to show\n@@ -633,8 +674,12 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co\n \t\t\tif (exclude && !(dir->flags & DIR_SHOW_IGNORED))\n \t\t\t\tcontinue;\n \n+\t\t\t/*\n+\t\t\t * The 'dtype' information comes from the filesystem,\n+\t\t\t * and we use the filesystem pathname for that (newpath)\n+\t\t\t */\n \t\t\tif (dtype == DT_UNKNOWN)\n-\t\t\t\tdtype = get_dtype(de, fullname);\n+\t\t\t\tdtype = get_dtype(de, newpath);\n \n \t\t\t/*\n \t\t\t * Do we want to see just the ignored files?\n@@ -651,9 +696,12 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co\n \t\t\tdefault:\n \t\t\t\tcontinue;\n \t\t\tcase DT_DIR:\n-\t\t\t\tmemcpy(fullname + baselen + len, \"/\", 2);\n-\t\t\t\tlen++;\n-\t\t\t\tswitch (treat_directory(dir, fullname, baselen + len, simplify)) {\n+\t\t\t\tmemcpy(newpath + len, FS_PATH_SEP, FS_PATH_SEP_LEN+1);\n+\t\t\t\tlen += FS_PATH_SEP_LEN;\n+\t\t\t\tmemcpy(newbase + nlen, \"/\", 2);\n+\t\t\t\tnlen++;\n+\n+\t\t\t\tswitch (treat_directory(dir, newbase, nlen, simplify)) {\n \t\t\t\tcase show_directory:\n \t\t\t\t\tif (exclude != !!(dir->flags\n \t\t\t\t\t\t\t& DIR_SHOW_IGNORED))\n@@ -661,7 +709,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co\n \t\t\t\t\tbreak;\n \t\t\t\tcase recurse_into_directory:\n \t\t\t\t\tcontents += read_directory_recursive(dir,\n-\t\t\t\t\t\tfullname, fullname, baselen + len, 0, simplify);\n+\t\t\t\t\t\tnewpath, newbase, nlen, 0, simplify);\n \t\t\t\t\tcontinue;\n \t\t\t\tcase ignore_directory:\n \t\t\t\t\tcontinue;\n@@ -675,7 +723,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co\n \t\t\tif (check_only)\n \t\t\t\tgoto exit_early;\n \t\t\telse\n-\t\t\t\tdir_add_name(dir, fullname, baselen + len);\n+\t\t\t\tdir_add_name(dir, newbase, nlen);\n \t\t}\n exit_early:\n \t\tclosedir(fdir);\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex e4eb8fa..457b529 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -534,7 +534,9 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,\n \tmemset(&d, 0, sizeof(d));\n \tif (o->dir)\n \t\td.exclude_per_dir = o->dir->exclude_per_dir;\n-\ti = read_directory(&d, ce->name, pathbuf, namelen+1, NULL);\n+\n+\t/* FIXME! Filesystem pathname vs internal git pathname! */\n+\ti = read_directory(&d, pathbuf, pathbuf, namelen+1, NULL);\n \tif (i)\n \t\treturn o->gently ? -1 :\n \t\t\terror(ERRORMSG(o, not_uptodate_dir), ce->name);\n-- \n1.6.3.1.11.g97114\n"},{"id":"113964","messageId":"alpine.LFD.2.01.0905141413080.3343@localhost.localdomain","threadId":"19352","inReplyTo":"alpine.LFD.2.01.0905141346440.3343@localhost.localdomain","subject":"Re: [PATCH 3/3] read_directory(): infrastructure for pathname character set conversion","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-14T21:23:37Z","receivedAt":"2009-05-14T21:23:37Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 14 May 2009, Linus Torvalds wrote:\n\n> In particular, this allows:\n> \n> - the filesystem path component separator to be set to something\n> different than the normal UNIX '/' character.\n\nI forgot to mention that this also now allows really having a different \nprefix. The old code had \"path\" and \"base\", and without really reading the \ncode you might think that you could have a different base for the two, but \nimmediately when it recursed, it would re-set the path and base to be the \nsame thing, so you could never really have two different address spaces.\n\nThe new code very much intentionally keeps the two apart, and the \n_intention_ is that on platforms like Windows, you should not just be able \nto use other path component separators like '\\', it should also be \npossible to use an absolute base (which, if I recall correctly, is the \nonly way to handle things like long path-names. But maybe I'm wrong - I \nreally don't know the crazy native Windows API's).\n\nIOW, the _intention_ is that you could literally pass in something like\n\n\t\"c:\\Source\\git\\myrepo\"\n\nas the \"path\", and with an empty \"base\", it would then be possible to \nbasically traverse the tree with the filesystem operations building up a \n\"path\" like\n\n\tc:\\Source\\git\\myrepo\\subdir\\myfile.txt\n\nwhile \"base\" would track it, but become \"subdir/myfile.txt\".\n\nIn fact, my intention was that the pathname could easily be in some crazy \nUTF16LE format (ie not a real \"string\" at all), but I might need to pass \nthe \"pathlen\" around as a parameter if we need to handle strings that \ncontain embedded NUL characters. That's an easy thing to do if required, \nthough.\n\nNow, it's possible that nobody wants to do that kind of crazy windows \nstuff, because even windows people are perfectly fine using regular utf-8. \nI really dunno. My point is more that this is meant to be very flexible \nbasic infrastructure and that we _could_ do things like that.\n\n\t\tLinus\n"},{"id":"113967","messageId":"alpine.DEB.1.00.0905150018070.26154@pacific.mpi-cbg.de","threadId":"19352","inReplyTo":"alpine.LFD.2.01.0905141346440.3343@localhost.localdomain","subject":"Re: [PATCH 3/3] read_directory(): infrastructure for pathname character set conversion","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-14T22:19:38Z","receivedAt":"2009-05-14T22:19:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 14 May 2009, Linus Torvalds wrote:\n\n> The use of \"//\" as the filesystem path component separator may be odd,\n\nHopefully it will not bite us on Windows: \"//fileserver/x\" is different \nfrom \"/fileserver/x\" there: the former tries to access the share \"x\" of \nsamba server \"fileserver\", while the latter will expand to \"C:\\Program \nFiles\\Git\\fileserver\\x\" (or wherever you installed Git).\n\nCiao,\nDscho\n"},{"id":"113971","messageId":"727e50150905141536r5f3c4c1ap615166ba71018bf3@mail.gmail.com","threadId":"19352","inReplyTo":"alpine.DEB.1.00.0905150018070.26154@pacific.mpi-cbg.de","subject":"Re: [PATCH 3/3] read_directory(): infrastructure for pathname character set conversion","fromName":"Aaron Cohen","fromEmail":"remleduff@gmail.com","sentAt":"2009-05-14T22:36:24Z","receivedAt":"2009-05-14T22:36:24Z","isPatch":true,"sender":{"key":"remleduff@gmail.com","avatar":null},"body":"On Thu, May 14, 2009 at 6:19 PM, Johannes Schindelin\n wrote:\n> Hi,\n>\n> On Thu, 14 May 2009, Linus Torvalds wrote:\n>\n>> The use of \"//\" as the filesystem path component separator may be odd,\n>\n> Hopefully it will not bite us on Windows: \"//fileserver/x\" is different\n> from \"/fileserver/x\" there: the former tries to access the share \"x\" of\n> samba server \"fileserver\", while the latter will expand to \"C:\\Program\n> Files\\Git\\fileserver\\x\" (or wherever you installed Git).\n>\n> Ciao,\n> Dscho\n\nDoes this possibly allow using the magic \"\\\\?\\\" prefix on windows to\navoid file name length restrictions?\n-- Aaron\n"},{"id":"113973","messageId":"alpine.LFD.2.01.0905141546220.3343@localhost.localdomain","threadId":"19352","inReplyTo":"alpine.DEB.1.00.0905150018070.26154@pacific.mpi-cbg.de","subject":"Re: [PATCH 3/3] read_directory(): infrastructure for pathname character set conversion","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-14T22:47:03Z","receivedAt":"2009-05-14T22:47:03Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 15 May 2009, Johannes Schindelin wrote:\n> \n> On Thu, 14 May 2009, Linus Torvalds wrote:\n> \n> > The use of \"//\" as the filesystem path component separator may be odd,\n> \n> Hopefully it will not bite us on Windows: \"//fileserver/x\" is different \n> from \"/fileserver/x\" there: the former tries to access the share \"x\" of \n> samba server \"fileserver\", while the latter will expand to \"C:\\Program \n> Files\\Git\\fileserver\\x\" (or wherever you installed Git).\n\nIt only does it in the middle of names, so it should be safe. \n\nThat said, it's also meant to be just a temporary debugging aid. I'm fine \nwith the '//' part not being merged.\n\n\t\t\tLinus\n"},{"id":"113975","messageId":"alpine.LFD.2.01.0905141547480.3343@localhost.localdomain","threadId":"19352","inReplyTo":"727e50150905141536r5f3c4c1ap615166ba71018bf3@mail.gmail.com","subject":"Re: [PATCH 3/3] read_directory(): infrastructure for pathname character set conversion","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-14T22:51:46Z","receivedAt":"2009-05-14T22:51:46Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 14 May 2009, Aaron Cohen wrote:\n> \n> Does this possibly allow using the magic \"\\\\?\\\" prefix on windows to\n> avoid file name length restrictions?\n\nThat would be the intention - eventually. The point being exactly that the \n'path' side can be done differently from the 'basename' part that git then \nuses internally.\n\nHowever, the thing is not complete. As shown from the strace, almost all \nfilesystem operations then end up using the 'git internal' name anyway. \nIt's currently literally just the filesystem traversal itself that knows \nto separate the notion of 'internal pathname representation' from the \nfilesystem accesses.\n\nSo right now, the only thing that uses the filesystem-specific stuff is \nthe \"opendir()\" (and the lstat() in case the filesystem doesn't support \nd_type in the dirent).\n\n\t\tKubys\n"},{"id":"114016","messageId":"alpine.LFD.2.01.0905151156090.3343@localhost.localdomain","threadId":"19352","inReplyTo":"alpine.LFD.2.01.0905141346440.3343@localhost.localdomain","subject":"[PATCH 4/3] Introduce 'convert_path_to_git()'","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-15T19:01:29Z","receivedAt":"2009-05-15T19:01:29Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nOk, this is at the stage where if somebody wants UTF-8 filenames to work \nsanely on OS X, you can now add just a couple of lines (assuming you find \nthe right conversion function to turn it into NFC), and get a pretty \nefficient end result.\n\nIt avoids the conversion overhead for the case where the filename is \nperfectly normal US-ASCII, so in 99% of all cases you'd not have to do \nanything fancy, and then if you have a slightly more expensive thing to \nhandle decomposed -> composed UTF-8, you'll still be ok.\n\nThere's a comment on \"This is where we should get fancy. Some day\", and \nthe only case that matters for OS X is \"convert_path_to_git()\", since you \nnever need to do it the other way around.\n\nAnybody with OS X, and a wish to be able to sanely use non-ASCII UTF-8?\n\n(I only did the \"unsigned long at a time\" for x86, which does unaligneds \nwell. If your architecture sucks at unaligned accesses, you don't want to \ndo that thing.)\n\n\t\tLinus\n\n---\nAuthor: Linus Torvalds \nDate: Fri May 15 09:40:57 2009 -0700\n\nAdd initial support for pathname conversion to UTF-8\n\nWe're still not converting anything, but this adds the infrastructure\nfor it, including a fast-path to handle the trivial case of all-ASCII\ncharacters with no need for conversion.\n\nThat means that we can then afford to spend a bit more time on the case\nwhere conversion fails.\n\nSigned-off-by: Linus Torvalds \n---\n Makefile | 1 +\n dir.c | 11 ++-----\n dir.h | 3 ++\n filename.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 93 insertions(+), 8 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 6e21643..fc847c7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -473,6 +473,7 @@ LIB_OBJS += editor.o\n LIB_OBJS += entry.o\n LIB_OBJS += environment.o\n LIB_OBJS += exec_cmd.o\n+LIB_OBJS += filename.o\n LIB_OBJS += fsck.o\n LIB_OBJS += graph.o\n LIB_OBJS += grep.o\ndiff --git a/dir.c b/dir.c\nindex 2d53b11..3c5c6a6 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -589,13 +589,6 @@ static int get_dtype(struct dirent *de, const char *path)\n \treturn dtype;\n }\n \n-/* No actual conversion yet */\n-static int convert_path_to_git(const char *path, int plen, char *result)\n-{\n-\tmemcpy(result, path, plen+1);\n-\treturn plen;\n-}\n-\n /*\n * For testing!\n *\n@@ -650,7 +643,9 @@ static int read_directory_recursive(struct dir_struct *dir,\n \t\t\t\tcontinue;\n \t\t\tmemcpy(newpath + pathlen, de->d_name, len+1);\n \n-\t\t\tnlen = convert_path_to_git(de->d_name, len, converted);\n+\t\t\tnlen = convert_path_to_git(de->d_name, len, converted, sizeof(converted));\n+\t\t\tif (nlen <= 0)\n+\t\t\t\tcontinue;\n \t\t\tif (nlen + baselen + 8 > sizeof(newbase))\n \t\t\t\tcontinue;\n \t\t\tmemcpy(newbase + baselen, converted, nlen+1);\ndiff --git a/dir.h b/dir.h\nindex f9d69dd..c04eb3a 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -66,6 +66,9 @@ struct dir_struct {\n #define MATCHED_EXACTLY 3\n extern int match_pathspec(const char **pathspec, const char *name, int namelen, int prefix, char *seen);\n \n+extern int convert_path_to_git(const char *, int, char *, int);\n+extern int convert_git_to_path(const char *, int, char *, int);\n+\n extern int fill_directory(struct dir_struct *dir, const char **pathspec);\n extern int read_directory(struct dir_struct *, const char *path, const char *base, int baselen, const char **pathspec);\n \ndiff --git a/filename.c b/filename.c\nnew file mode 100644\nindex 0000000..8a049ab\n--- /dev/null\n+++ b/filename.c\n@@ -0,0 +1,86 @@\n+/*\n+ * Filename character set conversion\n+ */\n+#include \"cache.h\"\n+#include \"dir.h\"\n+\n+#if defined(__x86_64__) || defined(__i386__)\n+#define FAST_UNALIGNED\n+#endif\n+\n+/*\n+ * The \"common\" case that requires no conversion: all 7-bit ASCII.\n+ *\n+ * Return how many character were trivially converted, negative on\n+ * error (result wouldn't fit in buffer even trivially).\n+ */\n+static int convert_path_common(const char *path, int plen, char *result, int resultlen)\n+{\n+\tint retval;\n+\n+\tif (plen+1 > resultlen)\n+\t\treturn -1;\n+\tretval = 0;\n+#ifdef FAST_UNALIGNED\n+\twhile (plen >= sizeof(unsigned long)) {\n+\t\tunsigned long x = *(unsigned long *)path;\n+\t\tif (x & (unsigned long) 0x8080808080808080ull)\n+\t\t\tbreak;\n+\t\t*(unsigned long *)result = x;\n+\t\tpath += sizeof(unsigned long);\n+\t\tresult += sizeof(unsigned long);\n+\t\tplen -= sizeof(unsigned long);\n+\t\tretval += sizeof(unsigned long);\n+\t}\n+#endif\n+\twhile (plen > 0) {\n+\t\tunsigned char x = *path;\n+\t\tif (x & 0x80)\n+\t\t\tbreak;\n+\t\t*result = x;\n+\t\tpath++;\n+\t\tresult++;\n+\t\tplen--;\n+\t\tretval++;\n+\t}\n+\t*result = 0;\n+\treturn retval;\n+}\n+\n+int convert_path_to_git(const char *path, int plen, char *result, int resultlen)\n+{\n+\tint retval;\n+\n+\tretval = convert_path_common(path, plen, result, resultlen);\n+\t/* Absolute failure, or total success.. */\n+\tif (retval < 0 || retval == plen)\n+\t\treturn retval;\n+\n+\t/* Skip the part we already did trivially */\n+\tresult += retval;\n+\tpath += retval;\n+\tplen -= retval;\n+\n+\t/* This is where we should get fancy. Some day */\n+\tmemcpy(result, path, plen+1);\n+\treturn retval + plen;\n+}\n+\n+int convert_git_to_path(const char *path, int plen, char *result, int resultlen)\n+{\n+\tint retval;\n+\n+\tretval = convert_path_common(path, plen, result, resultlen);\n+\t/* Absolute failure, or total success.. */\n+\tif (retval < 0 || retval == plen)\n+\t\treturn retval;\n+\n+\t/* Skip the part we already did trivially */\n+\tresult += retval;\n+\tpath += retval;\n+\tplen -= retval;\n+\n+\t/* This is where we should get fancy. Some day */\n+\tmemcpy(result, path, plen+1);\n+\treturn retval + plen;\n+}\n"},{"id":"114075","messageId":"7vy6sxpn2q.fsf@alter.siamese.dyndns.org","threadId":"19352","inReplyTo":"alpine.LFD.2.01.0905151156090.3343@localhost.localdomain","subject":"Re: [PATCH 4/3] Introduce 'convert_path_to_git()'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-16T06:40:45Z","receivedAt":"2009-05-16T06:40:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> +#ifdef FAST_UNALIGNED\n> +\twhile (plen >= sizeof(unsigned long)) {\n> +\t\tunsigned long x = *(unsigned long *)path;\n> +\t\tif (x & (unsigned long) 0x8080808080808080ull)\n> +\t\t\tbreak;\n> +\t\t*(unsigned long *)result = x;\n> +\t\tpath += sizeof(unsigned long);\n> +\t\tresult += sizeof(unsigned long);\n> +\t\tplen -= sizeof(unsigned long);\n> +\t\tretval += sizeof(unsigned long);\n> +\t}\n> +#endif\n\nIt somehow bothers me that the stride of this loop is protected from\nhaving different size of unsigned long from what the author of this\nfunction expected, but the literal constant used as the mask is not quite,\nwhich means that taken as a whole it does not work if your unsigned long\nis more than 8-bytes. No, I do not think it matters in practice, and I\nfind it a neat trick to cast the unsigned long long literal down to\nunsigned long, but still it looks somewhat, eh, what would I say...\n\n\"Ugly\" is not quite the word I am looking for. \"My gut feels that there\nhas to be a way to write this more cleanly, but I am frustrated that I\ncannot come up with one\" might be the word...\n"},{"id":"114105","messageId":"alpine.LFD.2.01.0905161008190.3301@localhost.localdomain","threadId":"19352","inReplyTo":"7vy6sxpn2q.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/3] Introduce 'convert_path_to_git()'","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-16T17:27:33Z","receivedAt":"2009-05-16T17:27:33Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 15 May 2009, Junio C Hamano wrote:\n> \n> \"Ugly\" is not quite the word I am looking for. \"My gut feels that there\n> has to be a way to write this more cleanly, but I am frustrated that I\n> cannot come up with one\" might be the word...\n\nWell, we can certainly make it even more interesting, and more prone to \nwork even when the word-size grows.\n\n\t#define MAX_SHIFT (8*sizeof(unsigned long))\n\t#define SHIFT_BITS(x,y) ((x) << ((y) & (MAX_SHIFT-1)))\n\n\t#define EXPAND(x,bits) ((x) | SHIFT_BITS(x,bits))\n\t#define EXPAND2(x,bits) EXPAND(EXPAND(x,bits),bits*2)\n\t#define EXPAND4(x,bits) EXPAND2(EXPAND2(x,bits),bits*4)\n\t\n\t#define MASK80 EXPAND4(0x80808080ul,32)\n\nand now it should work up to 256 bits without warnings or undefined \nbehavior (shifting by the word-size or more is not well-specified, which \nis why it has the \"MAX_SHIFT/SHIFT_BIT\" magic)\n\nUntested, of course. But it seems to work on 32-bit and 64-bit cases. I \ncan only hope that it works for 128-bit and 256-bit cases too.\n\nAnd yes, it depends on \"sizeof(unsigned long)\" being a power of two. We \ncould avoid that dependency by turning the \"& (MAX_SHIFT-1)\" into a ?: \noperation that actually compares with the value, and then it would work \nfor a 6-byte \"unsigned long\" too.\n\nIt fundamentally does depend on 8-bit bytes, of course, but so does the \nwhole algorithm, so that's not much of a dependency.\n\nIOW, I'm not claiming it's \"truly portable\". Just reasonably so.\n\n\t\t\tLinus\n"},{"id":"114258","messageId":"loom.20090519T120452-71@post.gmane.org","threadId":"19352","inReplyTo":"7vy6sxpn2q.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/3] Introduce 'convert_path_to_git()'","fromName":"Jens Kilian","fromEmail":"jjk@acm.org","sentAt":"2009-05-19T12:20:54Z","receivedAt":"2009-05-19T12:20:54Z","isPatch":true,"sender":{"key":"jjk@acm.org","avatar":null},"body":"Junio C Hamano pobox.com> writes:\n> \"Ugly\" is not quite the word I am looking for. \"My gut feels that there\n> has to be a way to write this more cleanly, but I am frustrated that I\n> cannot come up with one\" might be the word...\n\nHow about this:\n\n#include \n\n#define MAGIC(type) ((~(type)0 / (type)0xff) << 7)\n#define TEST(type) printf(#type \" %llx\\n\", (unsigned long long)MAGIC(type))\n\nint\nmain(void)\n{\n/*TEST(unsigned char); Doesn't work, and I'm too lazy to find out why. */\n TEST(unsigned int);\n TEST(unsigned long);\n TEST(unsigned long long);\n return 0;\n}\n\nHTH,\n Jens.\n"},{"id":"114264","messageId":"1242739908.17490.5.camel@cp-jk-linux.corp.on2.com","threadId":"19352","inReplyTo":"loom.20090519T120452-71@post.gmane.org","subject":"Re: [PATCH 4/3] Introduce 'convert_path_to_git()'","fromName":"John Koleszar","fromEmail":"john.koleszar@on2.com","sentAt":"2009-05-19T13:31:48Z","receivedAt":"2009-05-19T13:31:48Z","isPatch":true,"sender":{"key":"john.koleszar@on2.com","avatar":null},"body":"Haven't been following this thread, so I don't know what the context is\nhere, but the brain teaser caught my eye.\n\nOn Tue, 2009-05-19 at 08:20 -0400, Jens Kilian wrote:\n> Junio C Hamano pobox.com> writes:\n> > \"Ugly\" is not quite the word I am looking for. \"My gut feels that there\n> > has to be a way to write this more cleanly, but I am frustrated that I\n> > cannot come up with one\" might be the word...\n> \n> How about this:\n> \n> #include \n> \n\n-#define MAGIC(type) ((~(type)0 / (type)0xff) << 7)\n+#define MAGIC(type) ((type)(~(type)0 / 0xffU << 7))\n\n> #define TEST(type) printf(#type \" %llx\\n\", (unsigned long long)MAGIC(type))\n> \n> int\n> main(void)\n> {\n> /*TEST(unsigned char); Doesn't work, and I'm too lazy to find out why. */\n> TEST(unsigned int);\n> TEST(unsigned long);\n> TEST(unsigned long long);\n> return 0;\n> }\n> \n\n--John\n"}]}