views:

45

answers:

0

I have some code that I have inherited which is part of a class for iterating, accessing the directory content and uses boost::filesystem::path. The code reads in part:

struct directory_iterator_impl
{
private:
  void skip_dots(const path& p, const char* who)
  {
    static const std::string d(".");
    static const std::string dd("..");
    ASSERT( m_current_entry != 0 );
    while ( m_current_entry != 0 &&
        (m_current_entry->d_name == d ||
         m_current_entry->d_name == dd) ) {
      read(p, who);
    }
  }

  void handle_read_error(const path& p, const char* who)
  {
    ASSERT( errno != EBADF );
    ASSERT( errno != ENOENT );
    switch ( errno ) {
    case EOVERFLOW:
    case ESTALE:
      throw_error(who,
        error_code_impl::expected_unix_error(errno), p);
      break;
    default:
      base::utilities::abort_with_unknown_error();
      throw_error(who,
          error_code_impl::unexpected_unix_error(errno), p);
    }
  }

  void read(const path& p, const char* who)
  {
    errno = 0;
    m_current_entry = readdir(m_directory_stream);
    if ( errno != 0 ) {
      handle_read_error(p, who);
    }
  }

  void sync_current()
  {
    ASSERT( m_current_entry != 0 );
    m_current_name = m_current_entry->d_name;
  }

  void handle_open_error(const path& p, const char* who)
  {
    ASSERT( errno != EFAULT );
    switch ( errno ) {
    case EACCES: 
    case ELOOP:
    case ENAMETOOLONG:
    case ENOENT:
    case ENOTDIR:
    case EMFILE:
    case ENFILE:
    case ENOMEM:
    case ESTALE:
      throw_error(who,
          error_code_impl::expected_unix_error(errno), p);
      break;
    default:
      base::utilities::abort_with_unknown_error();
      throw_error(who,
          error_code_impl::unexpected_unix_error(errno), p);
    }
  }

public:
  directory_iterator_impl(const path& p, const char* w)
  {
    m_directory_stream = opendir(p.as_string().c_str());
    if (m_directory_stream == 0) {
      handle_open_error(p, w);
    }
    unsafe_increment(p, w);
  }

  ~directory_iterator_impl()
  {
    do {
      // coverity[double_free]
      if ( (m_directory_stream != 0) && 
        (closedir(m_directory_stream)==0) ) {
        return;
      }
    } while (errno == EINTR);
    ASSERT( errno != EBADF );
    base::utilities::abort_with_unknown_error();
  }

  void unsafe_increment(const path& p, const char* who)
  {
    read(p, who);
    if ( !no_entries() ) {
      skip_dots(p, who);
    }
    if ( !no_entries() ) {
      sync_current();
    }
  }

  void increment(const char* w)
  {
    ASSERT( !no_entries() );
    unsafe_increment("", w);
  }

  const std::string& dereference() const
  {
    return m_current_name;
  }

  bool no_entries() const
  {
    return m_current_entry == 0;
  }

private:
  DIR* m_directory_stream;
  struct dirent* m_current_entry;
  std::string m_current_name;
};

The issue I have run into is a non-reproducible ASSERT where errno == EBADF in handle_read_error(). Now inspecting the code, it appears to me that in the constructor, the m_directory_stream is set, and nothing else touches it. It is not NULL or the constructor would have called handle_open_error() and this case would not have arisen. Therefore, at the point of construction, the m_directory_stream was valid and no error occurred on open. However, some time later, unsafe_increment() is called, perhaps the call just later in the constructor, and at this point the DIR object is now EBADF and we get the assertion failure down the stack.

Here is information on the machine where the failure was detected (note the application is single threaded):

Linux 2.6.9-67.ELsmp x86_64
OS:        RedHat Enterprise Linux 4.0 U6
CPU: 8 x 3000 MHz, Intel(R)Xeon(R) (2 socket, quad core, No HT)
Memory: 16 GB RAM, 33 GB Swap

How can the file descriptor go bad while we are holding it? Is this a known problem with the dirent.h interface?

Please suggest how to make this code better and avoid this non-reproducible problem.