Forum: Ruby accidental Dir.glob behavior change in 1.8.6

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
B96c4e70956434cfc996c45edcdd9d54?d=identicon&s=25 Elliott Hughes (Guest)
on 2007-04-17 23:45
(Received via mailing list)
expected/1.8.4 behavior
-------------------

download ruby-1.8.4, extract, cd into the source root. (alternatively,
use Ubuntu 6.10's ruby package.)

./configure && make && ./ruby -w -e 'puts(Dir.glob("v*/config.h"))'

produces (after the build output) this output:

vms/config.h

actual 1.8.6 behavior
------------------

download ruby-1.8.6, extract, cd into the source root. (alternatively,
use Debian testing/unstable's ruby package.)

./configure && make && ./ruby -w -e 'puts(Dir.glob("v*/config.h"))'

produces (after the build output) this output:

-e:1: warning: variable.o/config.h: Not a directory
-e:1: warning: variable.c/config.h: Not a directory
-e:1: warning: version.c/config.h: Not a directory
-e:1: warning: version.o/config.h: Not a directory
-e:1: warning: version.h/config.h: Not a directory
vms/config.h

responsible code
----------------

in "dir.c":

static int
do_stat(const char *path, struct stat *pst, int flags)

{
    int ret = stat(path, pst);
    if (ret < 0 && errno != ENOENT)
        sys_warning(path);

    return ret;
}

static int
do_lstat(const char *path, struct stat *pst, int flags)
{
    int ret = lstat(path, pst);
    if (ret < 0 && errno != ENOENT)
        sys_warning(path);

    return ret;
}

static DIR *
do_opendir(const char *path, int flags)
{
    DIR *dirp = opendir(path);
    if (dirp == NULL && errno != ENOENT && errno != ENOTDIR)
        sys_warning(path);

    return dirp;
}

evaluation
-------------

stat(2)/lstat(2) can return ENOTDIR if a non-leaf element of the path
they're given is not a directory.
this is a fairly common case when people are using globbing as a cheap
way of finding files at a fixed depth below a given directory.

suggested fix
-----------------

make do_stat and do_lstat more like do_opendir:

-     if (ret < 0 && errno != ENOENT)
+     if (ret < 0 && errno != ENOENT && errno != ENOTDIR)

it might also be worth a code comment, since it's not likely to be
immediately obvious to most readers that stat(2)/lstat(2) can return
ENOTDIR.

thanks!

--
Elliott Hughes, BlueArc Engineering
F1d6cc2b735bfd82c8773172da2aeab9?d=identicon&s=25 Nobuyoshi Nakada (Guest)
on 2007-04-18 04:42
(Received via mailing list)
Hi,

At Wed, 18 Apr 2007 06:44:30 +0900,
Elliott Hughes wrote in [ruby-talk:248288]:
> evaluation
> -------------
>
> stat(2)/lstat(2) can return ENOTDIR if a non-leaf element of
> the path they're given is not a directory.
> this is a fairly common case when people are using globbing
> as a cheap way of finding files at a fixed depth below a
> given directory.

Thank you.

> it might also be worth a code comment, since it's not likely
> to be immediately obvious to most readers that
> stat(2)/lstat(2) can return ENOTDIR.

What about this?  The comment was borrowed from your post.


Index: dir.c
===================================================================
--- dir.c  (revision 12191)
+++ dir.c  (working copy)
@@ -929,4 +929,10 @@ sys_warning_1(const char* mesg)
 #define GLOB_JUMP_TAG(status) ((status == -1) ? rb_memerror() :
rb_jump_tag(status))

+/*
+ * ENOTDIR can return even if a non-leaf element of the path is not a
+ * directory.
+ */
+#define to_be_ignored(e) ((e) == ENOENT || (e) == ENOTDIR)
+
 /* System call with warning */
 static int
@@ -935,5 +941,5 @@ do_stat(const char *path, struct stat *p
 {
     int ret = stat(path, pst);
-    if (ret < 0 && errno != ENOENT)
+    if (ret < 0 && !to_be_ignored(errno))
   sys_warning(path);

@@ -945,5 +951,5 @@ do_lstat(const char *path, struct stat *
 {
     int ret = lstat(path, pst);
-    if (ret < 0 && errno != ENOENT)
+    if (ret < 0 && !to_be_ignored(errno))
   sys_warning(path);

@@ -955,5 +961,5 @@ do_opendir(const char *path, int flags)
 {
     DIR *dirp = opendir(path);
-    if (dirp == NULL && errno != ENOENT && errno != ENOTDIR)
+    if (dirp == NULL && !to_be_ignored(errno))
   sys_warning(path);
B96c4e70956434cfc996c45edcdd9d54?d=identicon&s=25 Elliott Hughes (Guest)
on 2007-04-18 04:48
(Received via mailing list)
looks good. i like the idea of factoring out the repeated test, too.
that's nicer than what i did.

actually i think you missed a couple of words in the comment. maybe
something like this?

+/*
+ * ENOTDIR can be returned by stat(2) if a non-leaf element of the path
+ * is not a directory.
+ */

btw, was this the right place to report a bug? there was no obvious link
on http://www.ruby-lang.org/en/ and searching that site for "bug
database" came back with 0 matches.

--
Elliott Hughes, BlueArc Engineering
F1d6cc2b735bfd82c8773172da2aeab9?d=identicon&s=25 Nobuyoshi Nakada (Guest)
on 2007-04-19 05:55
(Received via mailing list)
Hi,

At Wed, 18 Apr 2007 11:47:12 +0900,
Elliott Hughes wrote in [ruby-talk:248309]:
> actually i think you missed a couple of words in the comment. maybe something like this?
>
> +/*
> + * ENOTDIR can be returned by stat(2) if a non-leaf element of the path
> + * is not a directory.
> + */

Thank you.  I'll commit it if knu admits it.

> btw, was this the right place to report a bug? there was no
> obvious link on http://www.ruby-lang.org/en/ and searching
> that site for "bug database" came back with 0 matches.

Not wrong place, but ruby-core ML is better.
B96c4e70956434cfc996c45edcdd9d54?d=identicon&s=25 Elliott Hughes (Guest)
on 2007-04-26 04:02
(Received via mailing list)
i just noticed that you committed the change. i've checked out and
tested revision 12220 of ruby_1_8 and it's working perfectly. thank-you!

--
Elliott Hughes, BlueArc Engineering
This topic is locked and can not be replied to.