Accidental Dir.glob behavior change in 1.8.6

Ruby
1

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

2

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);
3

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 Ruby Programming Language and searching that site for “bug
database” came back with 0 matches.

–
Elliott Hughes, BlueArc Engineering

4

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 Ruby Programming Language and searching
that site for “bug database” came back with 0 matches.

Not wrong place, but ruby-core ML is better.

5

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