From ef11cd94e7c4e477b34cceaf4378e5a8c9218757 Mon Sep 17 00:00:00 2001 From: dearblue Date: Thu, 2 Jan 2025 17:42:40 +0900 Subject: [PATCH] Reimplement the `File.dirname` method The purpose is as follows: - Stop using `mrb_locale_from_utf8()`. - Because there is no corresponding `mrb_utf8_from_locale()`. - Because on Windows, for example, if the code page is 932 (CP932, likely ShiftJIS), it cannot be distinguished from the second byte 0x5c (\), and returns wrong results. - Stop using `dirname(3)`. - Because leading consecutive slashes are not truncated. For example, if `/////a/b` is given, CRuby returns `/a`, but mruby so far returns `/////a`. - Because the `path` argument cannot be passed in an immutable form. - Stop using `_splitpath()` in the Windows implementation. - Because there is no support for UNC paths with up to 32767 characters. ref. https://learn.microsoft.com/ja-jp/dotnet/standard/io/file-path-formats#unc-paths - Because modifying the result of paths terminated by a directory separator. Previously, for example, `C:/` would return `C:.` instead of `C:/`, and `a/b/` would return `a/b` instead of `a`. --- mrbgems/mruby-io/src/file.c | 82 ++++++++++++++++++++++------------- mrbgems/mruby-io/test/file.rb | 40 +++++++++++++++++ 2 files changed, 93 insertions(+), 29 deletions(-) diff --git a/mrbgems/mruby-io/src/file.c b/mrbgems/mruby-io/src/file.c index 589f1be14..eb0374955 100644 --- a/mrbgems/mruby-io/src/file.c +++ b/mrbgems/mruby-io/src/file.c @@ -51,8 +51,10 @@ #define PATH_SEPARATOR ";" #define FILE_ALT_SEPARATOR "\\" #define VOLUME_SEPARATOR ":" + #define DIRSEP_P(ch) (((ch) == '/') | ((ch) == '\\')) #else #define PATH_SEPARATOR ":" + #define DIRSEP_P(ch) ((ch) == '/') #endif #ifndef LOCK_SH @@ -163,42 +165,64 @@ mrb_file_s_rename(mrb_state *mrb, mrb_value obj) return mrb_fixnum_value(0); } +#define SKIP_DIRSEP(p) for (; DIRSEP_P(*(p)); (p)++) +#define NEXT_DIRSEP(p) for (; *(p) != '\0' && !DIRSEP_P(*(p)); (p)++) + +static const char* +scan_dirname(const char *path) +{ + const char *p = path + strlen(path); + for (; p > path && DIRSEP_P(p[-1]); p--) + ; + for (; p > path && !DIRSEP_P(p[-1]); p--) + ; + for (; p > path && DIRSEP_P(p[-1]); p--) + ; + return p > path ? p : path; +} + static mrb_value mrb_file_dirname(mrb_state *mrb, mrb_value klass) { -#if defined(_WIN32) - char dname[_MAX_DIR], vname[_MAX_DRIVE]; - char buffer[_MAX_DRIVE + _MAX_DIR]; - const char *utf8_path; - mrb_get_args(mrb, "z", &utf8_path); - char *path = mrb_locale_from_utf8(utf8_path, -1); - _splitpath(path, vname, dname, NULL, NULL); - snprintf(buffer, _MAX_DRIVE + _MAX_DIR, "%s%s", vname, dname); - mrb_locale_free(path); - size_t ridx = strlen(buffer); - if (ridx == 0) { - strncpy(buffer, ".", 2); /* null terminated */ - } - else if (ridx > 1) { - ridx--; - while (ridx > 0 && (buffer[ridx] == '/' || buffer[ridx] == '\\')) { - buffer[ridx] = '\0'; /* remove last char */ - ridx--; + const char *path; + mrb_get_args(mrb, "z", &path); + + const char *p = path; +#ifdef _WIN32 + if (DIRSEP_P(p[0]) && DIRSEP_P(p[1])) { + p += 2; + SKIP_DIRSEP(p); + path = p - 2; /* if consecutive, point to the trailing slash */ + NEXT_DIRSEP(p); + const char *o = p; + SKIP_DIRSEP(p); + if (*p == '\0') { + p = o; } + else { + NEXT_DIRSEP(p); + p = scan_dirname(p); + } + return mrb_str_new(mrb, path, p - path); } - return mrb_str_new_cstr(mrb, buffer); -#else - mrb_value s; - mrb_get_args(mrb, "S", &s); - char *path = mrb_locale_from_utf8(mrb_str_to_cstr(mrb, s), -1); - char *dname; - if ((dname = dirname(path)) == NULL) { - mrb_locale_free(path); - mrb_sys_fail(mrb, "dirname"); + else if (ISALPHA(p[0]) && p[1] == ':') { + p += 2; + const char *o = p; + SKIP_DIRSEP(p); + p = scan_dirname(p); + mrb_value s = mrb_str_new(mrb, path, p - path); + if (p == o) { + mrb_str_cat_lit(mrb, s, "."); + } + return s; } - mrb_locale_free(path); - return mrb_str_new_cstr(mrb, dname); #endif + SKIP_DIRSEP(p); + if (p > path) { + path = p - 1; /* if consecutive, point to the trailing slash */ + } + p = scan_dirname(p); + return (p == path) ? mrb_str_new_lit(mrb, ".") : mrb_str_new(mrb, path, p - path); } static mrb_value diff --git a/mrbgems/mruby-io/test/file.rb b/mrbgems/mruby-io/test/file.rb index 07e32b325..9ce3e4949 100644 --- a/mrbgems/mruby-io/test/file.rb +++ b/mrbgems/mruby-io/test/file.rb @@ -40,8 +40,48 @@ assert('File.dirname') do assert_equal '.', File.dirname('') assert_equal '.', File.dirname('a') assert_equal '/', File.dirname('/a') + assert_equal '/', File.dirname('/a/') assert_equal 'a', File.dirname('a/b') + assert_equal 'a', File.dirname('a/b/') + assert_equal 'a/b', File.dirname('a/b/c') assert_equal '/a', File.dirname('/a/b') + assert_equal '/a', File.dirname('/a/b/') + assert_equal '/a/b', File.dirname('/a/b/c') + assert_equal '/a/b', File.dirname('/a/b/c/') + assert_equal '/', File.dirname('/a//') + assert_equal '/a', File.dirname('/a//b') + assert_equal '/a/b', File.dirname('/a/b//c//') +end + +unless MRubyIOTestUtil.win? + assert('File.dirname (not Windows)') do + assert_equal '/a', File.dirname('//a//b/') + end +else + assert('File.dirname (on Windows)') do + assert_equal 'c:.', File.dirname('c:') + assert_equal 'c:.', File.dirname('c:a') + assert_equal 'c:.', File.dirname('c:a/') + assert_equal 'c:a', File.dirname('c:a/b') + assert_equal 'c:/', File.dirname('c:/') + assert_equal 'c:/', File.dirname('c:/a') + assert_equal 'c:/', File.dirname('c:/a/') + assert_equal 'c:/a', File.dirname('c:/a/b') + assert_equal '//.', File.dirname('//.') + assert_equal '//.', File.dirname('//./') + assert_equal '//./a', File.dirname('//./a') + assert_equal '//./a', File.dirname('//./a/') + assert_equal '//./a', File.dirname('//./a/b') + assert_equal '//./a/b', File.dirname('//./a/b/c') + assert_equal '//?', File.dirname('//?/') + assert_equal '//?/a', File.dirname('//?/a') + assert_equal '//?/a', File.dirname('//?/a/') + assert_equal '//?/a', File.dirname('//?/a/b') + assert_equal '//host1', File.dirname('//host1/') + assert_equal '//host1/a', File.dirname('//host1/a') + assert_equal '//host1/a', File.dirname('//host1/a/') + assert_equal '//host1/a', File.dirname('//host1/a/b') + end end assert('File.extname') do