Possible bug: DBG_FAIL triggered when dir.exists(filename) is called and filename does not exist

[cfeied]

when a file does not exist, checking for file existence returns false as expected. However, if I enable debugging it also triggers an unexpected DBG_FAIL before returning:

DBG_FAIL: FatFileLFN.cpp 418

the relevant code is:

create:
  // don't create unless O_CREAT and write mode
  if (!(oflag & O_CREAT) || !isWriteMode(oflag)) {
    DBG_FAIL_MACRO;
    goto fail;
  }

Although everything works fine with debugging turned off, this still seems like a possible bug because I can't see any reason why the code should be attempting to create a file when I am testing for file existence.

The initial call goes to FsFile.h at line 98:

  bool exists(const char* path) {
    return m_fFile ? m_fFile->exists(path) :
           m_xFile ? m_xFile->exists(path) : false;
  }

This is a Fat32 card, so the first branch is taken to FatFile.h at 258:

  bool exists(const char* path) {
    FatFile file;
    return file.open(this, path, O_RDONLY);
  }

This leads to FatFile.cpp at 412:

//------------------------------------------------------------------------------
bool FatFile::open(FatFile* dirFile, const char* path, oflag_t oflag) {
  FatFile tmpDir;
  fname_t fname;

  // error if already open
  if (isOpen() || !dirFile->isDir()) {
    DBG_FAIL_MACRO;
    goto fail;
  }
  if (isDirSeparator(*path)) {
    while (isDirSeparator(*path)) {
      path++;
    }
    if (*path == 0) {
      return openRoot(dirFile->m_vol);
    }
    if (!tmpDir.openRoot(dirFile->m_vol)) {
      DBG_FAIL_MACRO;
      goto fail;
    }
    dirFile = &tmpDir;
  }
  while (1) {
    if (!parsePathName(path, &fname, &path)) {
      DBG_FAIL_MACRO;
      goto fail;
    }
    if (*path == 0) {
      break;
    }
    if (!open(dirFile, &fname, O_RDONLY)) {
      DBG_FAIL_MACRO;
      goto fail;
    }
    tmpDir = *this;
    dirFile = &tmpDir;
    close();
  }
  return open(dirFile, &fname, oflag);

fail:
  return false;
}

Which leads finally to FatFileLFN.cpp at 302, with the DBG_FAIL occurring at line 418 at bottom:

//------------------------------------------------------------------------------
bool FatFile::open(FatFile* dirFile, fname_t* fname, oflag_t oflag) {
  bool fnameFound = false;
  uint8_t lfnOrd = 0;
  uint8_t freeNeed;
  uint8_t freeFound = 0;
  uint8_t order = 0;
  uint8_t checksum = 0;
  uint8_t ms10;
  uint16_t freeIndex = 0;
  uint16_t curIndex;
  uint16_t date;
  uint16_t time;
  DirFat_t* dir;
  DirLfn_t* ldir;
  size_t len = fname->len;

  if (!dirFile->isDir() || isOpen()) {
    DBG_FAIL_MACRO;
    goto fail;
  }
  // Number of directory entries needed.
  freeNeed = fname->flags & FNAME_FLAG_NEED_LFN ? 1 + (len + 12)/13 : 1;

  dirFile->rewind();
  while (1) {
    curIndex = dirFile->m_curPosition/32;
    dir = dirFile->readDirCache(true);
    if (!dir) {
      if (dirFile->getError()) {
        DBG_FAIL_MACRO;
        goto fail;
      }
      // At EOF
      goto create;
    }
    if (dir->name[0] == FAT_NAME_DELETED || dir->name[0] == FAT_NAME_FREE) {
      if (freeFound == 0) {
        freeIndex = curIndex;
      }
      if (freeFound < freeNeed) {
        freeFound++;
      }
      if (dir->name[0] == FAT_NAME_FREE) {
        goto create;
      }
    } else {
      if (freeFound < freeNeed) {
        freeFound = 0;
      }
    }
    // skip empty slot or '.' or '..'
    if (dir->name[0] == FAT_NAME_DELETED || dir->name[0] == '.') {
      lfnOrd = 0;
    } else if (isLongName(dir)) {
      ldir = reinterpret_cast<DirLfn_t*>(dir);
      if (!lfnOrd) {
        if ((ldir->order & FAT_ORDER_LAST_LONG_ENTRY) == 0) {
          continue;
        }
        lfnOrd = order = ldir->order & 0X1F;
        checksum = ldir->checksum;
      } else if (ldir->order != --order || checksum != ldir->checksum) {
        lfnOrd = 0;
        continue;
      }
      size_t k = 13*(order - 1);
      if (k >= len) {
        // Not found.
        lfnOrd = 0;
        continue;
      }
      for (uint8_t i = 0; i < 13; i++) {
        uint16_t u = lfnGetChar(ldir, i);
        if (k == len) {
          if (u != 0) {
            // Not found.
            lfnOrd = 0;
          }
          break;
        }
        if (u > 255 || lfnToLower(u) != lfnToLower(fname->lfn[k++])) {
          // Not found.
          lfnOrd = 0;
          break;
        }
      }
    } else if (isFileOrSubdir(dir)) {
      if (lfnOrd) {
        if (1 == order && lfnChecksum(dir->name) == checksum) {
          goto found;
        }
        DBG_FAIL_MACRO;
        goto fail;
      }
      if (!memcmp(dir->name, fname->sfn, sizeof(fname->sfn))) {
        if (!(fname->flags & FNAME_FLAG_LOST_CHARS)) {
          goto found;
        }
        fnameFound = true;
      }
    } else {
      lfnOrd = 0;
    }
  }


found:
  // Don't open if create only.
  if (oflag & O_EXCL) {
    DBG_FAIL_MACRO;
    goto fail;
  }
  goto open;

// ***********************************************
// **HERE IS WHERE THE DBG_FAIL OCCURS **
create:
  // don't create unless O_CREAT and write mode
  if (!(oflag & O_CREAT) || !isWriteMode(oflag)) {
    DBG_FAIL_MACRO;
    goto fail;
  }

[greiman]

I never intended for debug to be use for finding bugs in applications. It is intended to find bugs in the library. I try to place a debug macro everywhere a fail is returned and exists will return false.

The problem is that open will fail if you attempt to open a file that does not exist for read or write without O_CREATE.

There are many other quirks if you use debug to find bugs in applications.

[cfeied]

Yes, I was looking for bugs in the library because some of the results were not as expected. However, it turned out that the results were as I should have expected, had I understood the library better.

It just seemed odd that the library fires a debug_fail on the expected outcome of a function (testing for file existence returns false when the file is not present). It made me scrutinize the library code when in fact it was performing as designed.