Re: File.extname edge case bug?

mathew wrote:

> I think any string which *ends* with a "." should return "". I don't 
> think the start of the string should have anything to do with the output 
> of extname.

Based on this, which I think is correct, I submit the following patch.  I've 
also included a test suite and some benchmarks.  Sorry for any wrappage.

--- file.orig   Thu Aug 18 13:57:41 2005
+++ file.c      Tue Aug 23 16:08:10 2005
@@ -2664,22 +2664,18 @@
  rb_file_s_extname(klass, fname)
      VALUE klass, fname;
  {
-    char *name, *p, *e;
-    VALUE extname;
+   char* ptr;
+   VALUE rbExt = rb_str_new2(""); /* Default */

-    name = StringValueCStr(fname);
-    p = strrdirsep(name);      /* get the last path component */
-    if (!p)
-       p = name;
-    else
-       p++;
-
-     e = strrchr(p, '.');      /* get the last dot of the last component */
-     if (!e || e == p)         /* no dot, or the only dot is first? */
-        return rb_str_new2("");
-     extname = rb_str_new(e, chompdirsep(e) - e);      /* keep the dot, too! */
-     OBJ_INFECT(extname, fname);
-     return extname;
+   ptr = strrchr(StringValuePtr(rbStr), '.');
+
+   /* If '.' is present, not the last character, and not both the first
+    * and only '.' character, set the extname.
+   */
+   if(ptr && strlen(ptr) > 1 && strlen(ptr) != RSTRING(rbStr)->len)
+      rbExt = rb_str_new2(ptr);
+
+   return rbExt;
  }

  /*

# Test.rb
require "test/unit"

class TC_File_Extname < Test::Unit::TestCase
    def test_extname
       assert_equal(".rb", File.extname("foo.rb"))
       assert_equal(".rb", File.extname("/foo/bar.rb"))
       assert_equal(".rb", File.extname("c:\\foo\\bar.rb"))
       assert_equal(".c", File.extname("/foo.rb/bar.c"))
       assert_equal("", File.extname("bar"))
       assert_equal("", File.extname(".bashrc"))
       assert_equal(".conf", File.extname(".app.conf"))
       assert_equal(".conf", File.extname(".app.foo.conf"))
       assert_equal(".conf", File.extname("app.foo.conf"))
    end

    def test_extname_edge_cases
       assert_equal("", File.extname(""))
       assert_equal("", File.extname("."))
       assert_equal("", File.extname("/"))
       assert_equal("", File.extname("/."))
       assert_equal("", File.extname(".."))
    end

    def test_extname_expected_errors
       assert_raises(TypeError){ File.extname(nil) }
       assert_raises(TypeError){ File.extname(0) }
       assert_raises(TypeError){ File.extname(true) }
       assert_raises(TypeError){ File.extname(false) }
    end
end

# bench.rb - Temporarily created the 'Ext' class
# to distinguish new vs old versions.
require "ext"
require "benchmark"
include Benchmark

MAX = 100000

bm do |x|
    x.report("Old 1"){
       MAX.times{ File.extname("foo.rb") }
    }
    x.report("New 1"){
       MAX.times{ Ext.extname("foo.rb") }
    }

    puts

    x.report("Old 2"){
       MAX.times{ File.extname("foo") }
    }
    x.report("New 2"){
       MAX.times{ Ext.extname("foo") }
    }

    puts

    x.report("Old 3"){
       MAX.times{ File.extname(".foo") }
    }
    x.report("New 3"){
       MAX.times{ Ext.extname(".foo") }
    }

    puts

    x.report("Old 3"){
       MAX.times{ File.extname(".foo.conf") }
    }
    x.report("New 3"){
       MAX.times{ Ext.extname(".foo.conf") }
    }
end

Results on my system:

       user     system      total        real
Old 1  0.980000   0.450000   1.430000 (  1.534537)
New 1  1.140000   0.430000   1.570000 (  1.653467)

Old 2  0.970000   0.440000   1.410000 (  1.478212)
New 2  0.940000   0.440000   1.380000 (  1.427499)

Old 3  0.970000   0.440000   1.410000 (  1.469661)
New 3  0.970000   0.430000   1.400000 (  1.460048)

Old 3  0.970000   0.440000   1.410000 (  1.471304)
New 3  1.130000   0.440000   1.570000 (  1.637334)

So a tad slower in cases 1 and 4, while a smidgeon faster in case 2.

Regards,

Dan