From 3f3bf8a900d08aeca7058c017ff9cc3f9d2bd6eb Mon Sep 17 00:00:00 2001 From: Marcelo Roberto Jimenez Date: Sat, 10 Apr 2021 01:39:36 -0300 Subject: [PATCH 1/3] Fix for compiler warning --- src/SD.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SD.cpp b/src/SD.cpp index 24fcb2e..96d89c3 100644 --- a/src/SD.cpp +++ b/src/SD.cpp @@ -453,7 +453,7 @@ namespace SDLib { */ - int pathidx; + int pathidx = 0; // do the interactive search SdFile parentdir = getParentDir(filepath, &pathidx); From 3c3285dd61c31ddb176077161d55dea03afda88b Mon Sep 17 00:00:00 2001 From: Marcelo Roberto Jimenez Date: Sat, 10 Apr 2021 01:37:45 -0300 Subject: [PATCH 2/3] Explicitly initializes SdFile members and creates a destructor --- src/utility/SdFat.h | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/utility/SdFat.h b/src/utility/SdFat.h index 46b880b..939da74 100644 --- a/src/utility/SdFat.h +++ b/src/utility/SdFat.h @@ -151,7 +151,19 @@ uint16_t const FAT_DEFAULT_TIME = (1 << 11); class SdFile : public Print { public: /** Create an instance of SdFile. */ - SdFile(void) : type_(FAT_FILE_TYPE_CLOSED) {} + SdFile(void) + : flags_(0) + , type_(FAT_FILE_TYPE_CLOSED) + , curCluster_(0) + , curPosition_(0) + , dirBlock_(0) + , dirIndex_(0) + , fileSize_(0) + , firstCluster_(0) + , vol_(0) + {} + + virtual ~SdFile() {} /** writeError is set to true if an error occurs during a write(). Set writeError to false before calling print() and/or write() and check From 096d51578de76dd34607e1e03f1f0f8e9fd7a98d Mon Sep 17 00:00:00 2001 From: Marcelo Roberto Jimenez Date: Sat, 17 Mar 2018 23:26:13 -0300 Subject: [PATCH 3/3] Changes pointer to SdFile to SdFile member object The SD library was leaking memory due to the lack of destructor. There were subtle situations where temporary objects were created and destructed and the programmer had no chance to call close(). So, we need a destructor that calls close(). But to do so, we also need to change the pointer to SdFile to an actual SdFile object, otherwise the pointer will be copied and when one object is freed, the other object will loose its buffer too. --- src/File.cpp | 135 +++++++++++++++++++-------------------------------- src/SD.cpp | 6 +-- src/SD.h | 7 +-- 3 files changed, 57 insertions(+), 91 deletions(-) diff --git a/src/File.cpp b/src/File.cpp index 3e27fb4..1c80768 100644 --- a/src/File.cpp +++ b/src/File.cpp @@ -18,39 +18,45 @@ uint8_t nfilecount=0; */ -File::File(SdFile f, const char *n) { - // oh man you are kidding me, new() doesn't exist? Ok we do it by hand! - _file = (SdFile *)malloc(sizeof(SdFile)); - if (_file) { - memcpy(_file, &f, sizeof(SdFile)); - - strncpy(_name, n, 12); - _name[12] = 0; - - /* for debugging file open/close leaks - nfilecount++; - Serial.print("Created \""); - Serial.print(n); - Serial.print("\": "); - Serial.println(nfilecount, DEC); - */ - } -} - -File::File(void) { - _file = 0; +File::File(const SdFile &f, const char *n) +:Stream() +, _file(f) +, _name() +{ + strncpy(_name, n, 12); + _name[12] = 0; + /* for debugging file open/close leaks + nfilecount++; + Serial.print("Created \""); + Serial.print(n); + Serial.print("\": "); + Serial.println(nfilecount, DEC); + */ +} + +File::File(void) +:Stream() +, _file() +, _name() +{ _name[0] = 0; //Serial.print("Created empty file object"); } +File::~File(void) { + close(); + //Serial.print(F("Destructor called for file ")); + //Serial.println(_name); +} + // returns a pointer to the file name char *File::name(void) { return _name; } // a directory is a special type of file -bool File::isDirectory(void) { - return (_file && _file->isDir()); +boolean File::isDirectory(void) { + return _file.isDir(); } @@ -60,13 +66,10 @@ size_t File::write(uint8_t val) { size_t File::write(const uint8_t *buf, size_t size) { size_t t; - if (!_file) { - setWriteError(); - return 0; - } - _file->clearWriteError(); - t = _file->write(buf, size); - if (_file->getWriteError()) { + + _file.clearWriteError(); + t = _file.write(buf, size); + if (_file.getWriteError()) { setWriteError(); return 0; } @@ -74,95 +77,57 @@ size_t File::write(const uint8_t *buf, size_t size) { } int File::availableForWrite() { - if (_file) { - return _file->availableForWrite(); - } - return 0; + return _file.availableForWrite(); } int File::peek() { - if (! _file) { - return 0; - } - - int c = _file->read(); + int c = _file.read(); if (c != -1) { - _file->seekCur(-1); + _file.seekCur(-1); } return c; } int File::read() { - if (_file) { - return _file->read(); - } - return -1; + return _file.read(); } // buffered read for more efficient, high speed reading int File::read(void *buf, uint16_t nbyte) { - if (_file) { - return _file->read(buf, nbyte); - } - return 0; + return _file.read(buf, nbyte); } int File::available() { - if (! _file) { - return 0; - } - uint32_t n = size() - position(); return n > 0X7FFF ? 0X7FFF : n; } void File::flush() { - if (_file) { - _file->sync(); - } + _file.sync(); } -bool File::seek(uint32_t pos) { - if (! _file) { - return false; - } - - return _file->seekSet(pos); +boolean File::seek(uint32_t pos) { + return _file.seekSet(pos); } uint32_t File::position() { - if (! _file) { - return -1; - } - return _file->curPosition(); + return _file.curPosition(); } uint32_t File::size() { - if (! _file) { - return 0; - } - return _file->fileSize(); + return _file.fileSize(); } void File::close() { - if (_file) { - _file->close(); - free(_file); - _file = 0; - - /* for debugging file open/close leaks - nfilecount--; - Serial.print("Deleted "); - Serial.println(nfilecount, DEC); - */ - } + _file.close(); + /* for debugging file open/close leaks + nfilecount--; + Serial.print("Deleted "); + Serial.println(nfilecount, DEC); + */ } File::operator bool() { - if (_file) { - return _file->isOpen(); - } - return false; + return _file.isOpen(); } - diff --git a/src/SD.cpp b/src/SD.cpp index 96d89c3..361eedb 100644 --- a/src/SD.cpp +++ b/src/SD.cpp @@ -586,7 +586,7 @@ namespace SDLib { dir_t p; //Serial.print("\t\treading dir..."); - while (_file->readDir(&p) > 0) { + while (_file.readDir(&p) > 0) { // done if past last used entry if (p.name[0] == DIR_NAME_FREE) { @@ -609,7 +609,7 @@ namespace SDLib { // print file name with possible blank fill SdFile f; char name[13]; - _file->dirName(p, name); + _file.dirName(p, name); //Serial.print("try to open file "); //Serial.println(name); @@ -628,7 +628,7 @@ namespace SDLib { void File::rewindDirectory(void) { if (isDirectory()) { - _file->rewind(); + _file.rewind(); } } diff --git a/src/SD.h b/src/SD.h index c81a7d3..ccbc638 100644 --- a/src/SD.h +++ b/src/SD.h @@ -28,11 +28,12 @@ namespace SDLib { class File : public Stream { private: char _name[13]; // our name - SdFile *_file; // underlying file pointer + SdFile _file; // underlying file object public: - File(SdFile f, const char *name); // wraps an underlying SdFile + File(const SdFile & f, const char *name); // wraps an underlying SdFile File(void); // 'empty' constructor + virtual ~File(); // destructor virtual size_t write(uint8_t); virtual size_t write(const uint8_t *buf, size_t size); virtual int availableForWrite(); @@ -46,7 +47,7 @@ namespace SDLib { uint32_t size(); void close(); operator bool(); - char * name(); + char *name(); bool isDirectory(void); File openNextFile(uint8_t mode = O_RDONLY);