Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2.2.3 fails to compile #490

Open
teverth opened this issue Aug 10, 2024 · 10 comments
Open

2.2.3 fails to compile #490

teverth opened this issue Aug 10, 2024 · 10 comments

Comments

@teverth
Copy link

teverth commented Aug 10, 2024

Version 2.2.3 fails to compile on ESP32 board (or perhaps others too).

'FsFile::FsFile(const FsFile&)' is implicitly deleted because the default definition would be ill-formed

@greiman
Copy link
Owner

greiman commented Aug 10, 2024

I need a simple example for this problem. Which board and which version of the ESP32 board package.

My test examples compile with this version:

Screenshot 2024-08-10 065324

@greiman
Copy link
Owner

greiman commented Aug 10, 2024

You may be using the copy constructor for FsFile. It is explicitly deleted here.

It is deleted since filesystem corruption can occur if there are multiple instance of the file object for a file.

@teverth
Copy link
Author

teverth commented Aug 10, 2024

These are the compile time error messages:

C:\Users\Thomas\Documents\Arduino\Projects\CH4_Flux\CH4_Flux.ino: In function 'void DirectoryList()':
C:\Users\Thomas\Documents\Arduino\Projects\CH4_Flux\CH4_Flux.ino:711:17: error: use of deleted function 'FsFile::FsFile(const FsFile&)'
711 | printDirectory(root, 0);
| ~~~~~~~~~~~~~~^~~~~~~~~
In file included from c:\Users\Thomas\Documents\Arduino\libraries\SdFat\src/FsLib/FsLib.h:31,
from c:\Users\Thomas\Documents\Arduino\libraries\SdFat\src/SdFat.h:33,
from C:\Users\Thomas\Documents\Arduino\Projects\CH4_Flux\CH4_Flux.ino:34:
c:\Users\Thomas\Documents\Arduino\libraries\SdFat\src/FsLib/FsFile.h:918:7: note: 'FsFile::FsFile(const FsFile&)' is implicitly deleted because the default definition would be ill-formed:
918 | class FsFile : public StreamFile<FsBaseFile, uint64_t> {
| ^~~~~~
c:\Users\Thomas\Documents\Arduino\libraries\SdFat\src/FsLib/FsFile.h:918:7: error: use of deleted function 'StreamFile<FsBaseFile, long long unsigned int>::StreamFile(const StreamFile<FsBaseFile, long long unsigned int>&)'
In file included from c:\Users\Thomas\Documents\Arduino\libraries\SdFat\src/ExFatLib/ExFatFile.h:883,
from c:\Users\Thomas\Documents\Arduino\libraries\SdFat\src/ExFatLib/ExFatVolume.h:27,
from c:\Users\Thomas\Documents\Arduino\libraries\SdFat\src/ExFatLib/ExFatLib.h:28,
from c:\Users\Thomas\Documents\Arduino\libraries\SdFat\src/SdFat.h:31:
c:\users\thomas\documents\arduino\libraries\sdfat\src\common\arduinofiles.h:61:7: note: 'StreamFile<FsBaseFile, long long unsigned int>::StreamFile(const StreamFile<FsBaseFile, long long unsigned int>&)' is implicitly deleted because the default definition would be ill-formed:
61 | class StreamFile : public stream_t, public BaseFile {
| ^~~~~~~~~~
c:\users\thomas\documents\arduino\libraries\sdfat\src\common\arduinofiles.h:61:7: error: 'FsBaseFile::FsBaseFile(const FsBaseFile&)' is private within this context
c:\Users\Thomas\Documents\Arduino\libraries\SdFat\src/FsLib/FsFile.h:75:3: note: declared private here
75 | FsBaseFile(const FsBaseFile& from);
| ^~~~~~~~~~
C:\Users\Thomas\Documents\Arduino\Projects\CH4_Flux\CH4_Flux.ino:680:26: note: initializing argument 1 of 'void printDirectory(File, int)'
680 | void printDirectory(File dir, int numTabs) {
| ~~~~~^~~

exit status 1

Compilation error: use of deleted function 'FsFile::FsFile(const FsFile&)'

@teverth
Copy link
Author

teverth commented Aug 10, 2024

This is my initialisation code as per your examples:

// --------------- SDFat includes -------------------------------------------

#include "SdFat.h"

// SD_FAT_TYPE = 0 for SdFat/File as defined in SdFatConfig.h,
// 1 for FAT16/FAT32, 2 for exFAT, 3 for FAT16/FAT32 and exFAT.
#define SD_FAT_TYPE 0
/*
Change the value of SD_CS_PIN if you are using SPI and
your hardware does not use the default value, SS.
Common values are:
Arduino Ethernet shield: pin 4
Sparkfun SD shield: pin 8
Adafruit SD shields and modules: pin 10
*/

// SDCARD_SS_PIN is defined for the built-in SD on some boards.
#ifndef SDCARD_SS_PIN
const uint8_t SD_CS_PIN = SS;
#else // SDCARD_SS_PIN
// Assume built-in SD is used.
const uint8_t SD_CS_PIN = SDCARD_SS_PIN;
#endif // SDCARD_SS_PIN

// Try max SPI clock for an SD. Reduce SPI_CLOCK if errors occur.
#define SPI_CLOCK SD_SCK_MHZ(16)

// Try to select the best SD card configuration.
#if HAS_SDIO_CLASS
#define SD_CONFIG SdioConfig(FIFO_SDIO)
#elif ENABLE_DEDICATED_SPI
#define SD_CONFIG SdSpiConfig(SD_CS_PIN, DEDICATED_SPI, SPI_CLOCK)
#else // HAS_SDIO_CLASS
#define SD_CONFIG SdSpiConfig(SD_CS_PIN, SHARED_SPI, SPI_CLOCK)
#endif // HAS_SDIO_CLASS

#if SD_FAT_TYPE == 0
SdFat sd;
File dir;
File file;
#elif SD_FAT_TYPE == 1
SdFat32 sd;
File32 dir;
File32 file;
#elif SD_FAT_TYPE == 2
SdExFat sd;
ExFile dir;
ExFile file;
#elif SD_FAT_TYPE == 3
SdFs sd;
FsFile dir;
FsFile file;
#else // SD_FAT_TYPE
#error invalid SD_FAT_TYPE
#endif // SD_FAT_TYPE
//------------------------------------------------------------------------------
// Store error strings in flash to save RAM.
#define error(s) sd.errorHalt(&Serial, F(s))
//------------------------------------------------------------------------------

@teverth
Copy link
Author

teverth commented Aug 10, 2024

The board is a Adafruit ESP32 Feather.

The code as working well with your version 2.2.2

@teverth
Copy link
Author

teverth commented Aug 10, 2024

I did not alter SdFatConfig.h, in any way

@hlide
Copy link

hlide commented Aug 11, 2024

I had the same issue when I upgraded SdFat for my project. Since @greiman explained the rationale behind the deletion of copy constructor (to avoid potential corruption files), I can only accept the decision as it sounds pertinent. I simply replace this way:
dir[dir_depth] = entry; --------------> dir[dir_depth].move(&entry);

I could also use dir[dir_depth].copy(&entry); but as I don't need to keep entry as such (it will be the next subdirectory to enter), it won't make sense to do so and I believe having a copy may lead to the same potential corruption if badly coded.

@greiman
Copy link
Owner

greiman commented Aug 11, 2024

Functions like this with call by value cause problems.
void printDirectory(File dir, int numTabs)

The reason is that a copy of the first argument is made on the stack before a call. Any changes to the file are lost.

In the case of PrintDirectory the only changes such as file position may not mater. In cases where a file is open for write, the FAT table may be corrupted.

You can change the function to call by reference like this:
void printDirectory(File &dir, int numTabs)

Then only one instance of the file object will exist.

Try this example. Remove comment from call to listDirValue(root) to see the error.

#include "SdFat.h"
SdFs sd;
FsFile root;

void listDirValue(FsFile dir) {
  dir.ls();
}
void listDirReference(FsFile &dir) {
  dir.ls();
}

void setup() {
  Serial.begin(9600);
  sd.begin();
  root.open("/");
  listDirReference(root);
// Call by value won't compile
//  listDirValue(root);
}
void loop() {}

Here is test output from the above program:

bench.dat
bench0.txt
bench1.txt
bench2.txt
bench3.txt
bench4.txt
bench5.txt

Another way is to defeat the protection by editing SdFatConfig.h here to make the copy constructor public.

@greiman
Copy link
Owner

greiman commented Aug 11, 2024

I made the change since even a very skilled C++ programmer created a bug using call by value and suggested I protect from duplicate instances of the file object.

I agreed and it saves me from debugging programs that have filesystem corruption.

@teverth
Copy link
Author

teverth commented Aug 12, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants