Bug in readdir_r?

[fabbo]

I am using Haiku to build the following program

#include <stdlib.h>
#include <stdio.h>

#include <errno.h>
#include <dirent.h>

int main(void) {
  DIR *plugindir = opendir("loaders");
  struct dirent direntry;
  struct dirent *dirresult;
  int i;

  if (plugindir == NULL) {
    return;
  }
  do {
#ifdef CRASH
    int res = readdir_r(plugindir, &direntry, &dirresult);
    if (res != 0) {
     printf("fail %s\n",strerror(res));
      break;
    }
#else
  dirresult = readdir(plugindir);
#endif

    if (dirresult == NULL)
      break;

    printf("file %s\n",dirresult->d_name);
  } while (1);
  closedir(plugindir);
  return 0;
}

I use native gcc 2.95.3 (that is, no cross-compiling, gcc runs on Haiku itself) to build it (by the way, are newer version of gcc available?). If I compile it with -DCRASH, this is the output.

file .
fail Operation not supported
cr: /storage/haiku-releases/r1alpha4/haiku/src/system/libroot/posix/malloc/processheap.cpp:166:void BPrivate::processHeap::free(void *): sb
Kill Thread

[fabbo]

The same program runs fine on Linux, with and without -DCRASH

[hamish]

You are expected to allocate a dirent structure big enough for the names of the files in the directory. The d_name array in the struct definition only has space for one character. In this case, readdir_r scribbles over plugindir, causing the second call to readdir_r to fail and the call to closedir to crash.

The reason it works on Linux is because struct dirent has a larger d_name. But since POSIX doesn't specify the length of the array, portable code shouldn't rely on that.

[nielx]

I would like to reopen discussion on this issue, as I have recently been bitten by it as well. On all major platforms (Linux, *BSD, MacOS X) the definition of the dirent_t struct contains a name member that is the size of the max path plus one. It is not unlikely that other ported apps also depend on this behavior, and that there actually already are bugs in existing packages because of this.

[bonefish]

At least the ​Linux man page is pretty explicit about how to use readdir_r():

Since POSIX.1 does not specify the size of the d_name field, and other nonstandard fields may precede that field within the dirent structure, portable applications that use readdir_r() should allocate the buffer whose address is passed in entry as follows:

name_max = pathconf(dirpath, _PC_NAME_MAX);
if (name_max == -1)         /* Limit not defined, or error */
    name_max = 255;         /* Take a guess */
len = offsetof(struct dirent, d_name) + name_max + 1;
entryp = malloc(len);

That aside, it is not entirely trivial to change the size of the d_name field. There's quite a bit of code that would need to be reviewed (file systems, VFS, storage kit) and the (binary/source) compatibility implications need to be weighed as well.

[nielx]

Thanks for the backstory. I will upstream a patch that will follow that example.