From 1f7bc64845d71642845e5459dbf7e5450a616c66 Mon Sep 17 00:00:00 2001 From: dearblue Date: Mon, 21 Jul 2025 10:16:38 +0900 Subject: [PATCH 1/5] Added test for `File.basename` --- mrbgems/mruby-io/test/file.rb | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/mrbgems/mruby-io/test/file.rb b/mrbgems/mruby-io/test/file.rb index e6eda5434..fb5f16f5b 100644 --- a/mrbgems/mruby-io/test/file.rb +++ b/mrbgems/mruby-io/test/file.rb @@ -57,6 +57,40 @@ assert('File.basename with suffix') do assert_equal 'foo.rb', File.basename('foo.rb', '.RB') # case-sensitive end +if MRubyIOTestUtil.win? + assert('File.basename (for Windows)') do + assert_equal '/', File.basename('/') + assert_equal '/', File.basename('//a') + assert_equal '/', File.basename('//a/') + assert_equal '/', File.basename('//a/b') + assert_equal '/', File.basename('//a/b/') + assert_equal 'c', File.basename('//a/b/c') + assert_equal 'c', File.basename('//a/b/c/') + assert_equal '/', File.basename("\\\\a\\b") + assert_equal '', File.basename('c:') + assert_equal '/', File.basename('c:/') + assert_equal 'a', File.basename('c:/a') + assert_equal 'a', File.basename('c:/a/') + assert_equal 'b', File.basename('c:/a/b') + assert_equal '/', File.basename("c:\\") + end +else + assert('File.basename (for generic)') do + assert_equal '/', File.basename('/') + assert_equal 'a', File.basename('//a') + assert_equal 'a', File.basename('//a/') + assert_equal 'b', File.basename('//a/b') + assert_equal 'b', File.basename('//a/b/') + assert_equal 'c', File.basename('//a/b/c') + assert_equal 'c', File.basename('//a/b/c/') + assert_equal 'c:', File.basename('c:') + assert_equal 'c:', File.basename('c:/') + assert_equal 'a', File.basename('c:/a') + assert_equal 'a', File.basename('c:/a/') + assert_equal 'b', File.basename('c:/a/b') + end +end + assert('File.dirname') do assert_equal '.', File.dirname('') assert_equal '.', File.dirname('a') From f72a32ad87f4c64e9aca86762a6fce3bf381e05c Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jul 2025 20:51:47 +0900 Subject: [PATCH 2/5] Improvements to the Windows implementation of `File.basename` This change is mainly to remove `_splitpath()`. The reason is: - UNC paths are not supported. - The behavior is not guaranteed to be consistent when passing a UTF-8 string, since the handling of multibyte characters changes depending on the set codepage. - Only up to 255 bytes can be processed. Therefore, it is preferable to integrate with non-Windows implementations to ensure consistent implementation behavior. --- mrbgems/mruby-io/src/file.c | 63 +++++++++++++------------------------ 1 file changed, 22 insertions(+), 41 deletions(-) diff --git a/mrbgems/mruby-io/src/file.c b/mrbgems/mruby-io/src/file.c index 6210915ec..b2a23047c 100644 --- a/mrbgems/mruby-io/src/file.c +++ b/mrbgems/mruby-io/src/file.c @@ -241,44 +241,6 @@ mrb_file_dirname(mrb_state *mrb, mrb_value klass) static mrb_value mrb_file_basename(mrb_state *mrb, mrb_value klass) { -#if defined(_WIN32) - char bname[_MAX_DIR]; - char extname[_MAX_EXT]; - char *path; - const char *suffix = NULL; - - mrb_get_args(mrb, "z|z", &path, &suffix); - size_t ridx = strlen(path); - if (ridx > 0) { - ridx--; - while (ridx > 0 && (path[ridx] == '/' || path[ridx] == '\\')) { - path[ridx] = '\0'; - ridx--; - } - if (ridx == 0 && path[0] == '/') { - mrb_value result = mrb_str_new_cstr(mrb, path); - if (suffix && *suffix) { - mrb_int blen = RSTRING_LEN(result); - mrb_int slen = strlen(suffix); - if (blen > slen && memcmp(RSTRING_PTR(result) + blen - slen, suffix, slen) == 0) { - mrb_str_resize(mrb, result, blen - slen); - } - } - return result; - } - } - _splitpath((const char*)path, NULL, NULL, bname, extname); - mrb_value buffer = mrb_str_new_cstr(mrb, bname); - mrb_str_cat_cstr(mrb, buffer, extname); - if (suffix && *suffix) { - mrb_int blen = RSTRING_LEN(buffer); - mrb_int slen = strlen(suffix); - if (blen > slen && memcmp(RSTRING_PTR(buffer) + blen - slen, suffix, slen) == 0) { - mrb_str_resize(mrb, buffer, blen - slen); - } - } - return buffer; -#else char *path; const char *suffix = NULL; @@ -290,14 +252,34 @@ mrb_file_basename(mrb_state *mrb, mrb_value klass) return mrb_str_new_lit(mrb, "."); } +#ifdef _WIN32 + if (UNC_PATH_P(path)) { + const char *p = path + 2; + SKIP_DIRSEP(p); + NEXT_DIRSEP(p); // skip server name + SKIP_DIRSEP(p); + NEXT_DIRSEP(p); // skip share name + len -= p - path; + path = p; + } + else if (DRIVE_LETTER_P(path)) { + const char *p = path + 2; + if (p == path + len) { + return mrb_str_new_lit(mrb, ""); + } + len -= p - path; + path = p; + } +#endif // _WIN32 + // Remove trailing slashes (except when path is only "/") - while (len > 1 && path[len - 1] == '/') { + while (len > 1 && DIRSEP_P(path[len - 1])) { len--; } // Find the last path separator ssize_t base = len - 1; - while (base >= 0 && path[base] != '/') { + while (base >= 0 && !DIRSEP_P(path[base])) { base--; } base++; // move to the first character after '/' @@ -319,7 +301,6 @@ mrb_file_basename(mrb_state *mrb, mrb_value klass) } return result; -#endif } static mrb_value From 3f271336391d97db5c5ef5a5351667350dae356d Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jul 2025 21:01:06 +0900 Subject: [PATCH 3/5] Take a `const char *` for the "z" specifier of `mrb_get_args()` --- mrbgems/mruby-io/src/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mrbgems/mruby-io/src/file.c b/mrbgems/mruby-io/src/file.c index b2a23047c..973a68425 100644 --- a/mrbgems/mruby-io/src/file.c +++ b/mrbgems/mruby-io/src/file.c @@ -241,7 +241,7 @@ mrb_file_dirname(mrb_state *mrb, mrb_value klass) static mrb_value mrb_file_basename(mrb_state *mrb, mrb_value klass) { - char *path; + const char *path; const char *suffix = NULL; mrb_get_args(mrb, "z|z", &path, &suffix); From d230bb16d4724faa237005bd4dfb7e0e4ff7b21c Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jul 2025 21:28:35 +0900 Subject: [PATCH 4/5] Use end pointers instead of lengths For processing simplicity. --- mrbgems/mruby-io/src/file.c | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/mrbgems/mruby-io/src/file.c b/mrbgems/mruby-io/src/file.c index 973a68425..e842c482f 100644 --- a/mrbgems/mruby-io/src/file.c +++ b/mrbgems/mruby-io/src/file.c @@ -246,50 +246,44 @@ mrb_file_basename(mrb_state *mrb, mrb_value klass) mrb_get_args(mrb, "z|z", &path, &suffix); - // Copy path to a local buffer to avoid modifying the original string - size_t len = strlen(path); - if (len == 0) { + const char *endp = path + strlen(path); + if (path == endp) { return mrb_str_new_lit(mrb, "."); } #ifdef _WIN32 if (UNC_PATH_P(path)) { - const char *p = path + 2; - SKIP_DIRSEP(p); - NEXT_DIRSEP(p); // skip server name - SKIP_DIRSEP(p); - NEXT_DIRSEP(p); // skip share name - len -= p - path; - path = p; + path += 2; + SKIP_DIRSEP(path); + NEXT_DIRSEP(path); // skip server name + SKIP_DIRSEP(path); + NEXT_DIRSEP(path); // skip share name } else if (DRIVE_LETTER_P(path)) { - const char *p = path + 2; - if (p == path + len) { + path += 2; + if (path == endp) { return mrb_str_new_lit(mrb, ""); } - len -= p - path; - path = p; } #endif // _WIN32 // Remove trailing slashes (except when path is only "/") - while (len > 1 && DIRSEP_P(path[len - 1])) { - len--; + while (path < endp && DIRSEP_P(endp[-1])) { + endp--; } // Find the last path separator - ssize_t base = len - 1; - while (base >= 0 && !DIRSEP_P(path[base])) { + const char *base = endp; + while (path < base && !DIRSEP_P(base[-1])) { base--; } - base++; // move to the first character after '/' // If path is all slashes, return "/" - if ((size_t)base == len) { + if (base == endp) { return mrb_str_new_lit(mrb, "/"); } - mrb_value result = mrb_str_new(mrb, path + base, len - base); + mrb_value result = mrb_str_new(mrb, base, endp - base); // Suffix removal (CRuby compatible) if (suffix && *suffix) { From 40371ebf01f3eab89d4bbf927bdbdf1559dd085f Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jul 2025 21:29:44 +0900 Subject: [PATCH 5/5] Generate the string object last at the end of processing This is simpler than processing `suffix` after the string object has been created. --- mrbgems/mruby-io/src/file.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/mrbgems/mruby-io/src/file.c b/mrbgems/mruby-io/src/file.c index e842c482f..97990b37a 100644 --- a/mrbgems/mruby-io/src/file.c +++ b/mrbgems/mruby-io/src/file.c @@ -283,18 +283,16 @@ mrb_file_basename(mrb_state *mrb, mrb_value klass) return mrb_str_new_lit(mrb, "/"); } - mrb_value result = mrb_str_new(mrb, base, endp - base); - // Suffix removal (CRuby compatible) if (suffix && *suffix) { - mrb_int blen = RSTRING_LEN(result); + mrb_int blen = endp - base; mrb_int slen = strlen(suffix); - if (blen > slen && memcmp(RSTRING_PTR(result) + blen - slen, suffix, slen) == 0) { - mrb_str_resize(mrb, result, blen - slen); + if (blen > slen && memcmp(endp - slen, suffix, slen) == 0) { + endp -= slen; } } - return result; + return mrb_str_new(mrb, base, endp - base); } static mrb_value