You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Context: I am trying to hunt a bug, similar to #11 - working with older Arduino IDE, I know ;) but going deseperately through SD library's code I found cause for many subtle bugs.
On "normal" File, close() is idempotent (as expected), so doing close() multiple times is OK and 2nd+ calls do nothing. There seems to be no copy constructor in File, so when passing by-value to doSomething, the compiler generates (!) one that copies the fields - that is the _file pointer becomes shared between myFile and f locals.
First close will deallocate the (shared) _file pointer and zeroes the pts. The 2nd close will deallocate the same pointer value again.
This reduced example may seem bad, as the programmer should not do permature close - but lack of the copy/destroy semantics severely limits structuring the code since passing File values around easily leads to memory corruption. At this time, the only value safe to assign to other variables is File().
I would also recommmend to reconsider the decision for #2; while it is true, that the current implementation is simple, it requires extreme caution when passing around File variables. I'd recommend to have full copy+move constructor semantics.
If unwanted, I'd recommend to disallow generation by the compiler, so the user is warned: File(const File&) = delete;
Class name(argument) { File f = value ; // intialized value according to requirement };
Copy constructor is to made in class then ,we easily pass by value.
Context: I am trying to hunt a bug, similar to #11 - working with older Arduino IDE, I know ;) but going deseperately through SD library's code I found cause for many subtle bugs.
Consider code like this:
On "normal" File,
close()
is idempotent (as expected), so doing close() multiple times is OK and 2nd+ calls do nothing. There seems to be no copy constructor inFile
, so when passing by-value todoSomething
, the compiler generates (!) one that copies the fields - that is the_file
pointer becomes shared betweenmyFile
andf
locals.First
close
will deallocate the (shared)_file
pointer and zeroes the pts. The 2ndclose
will deallocate the same pointer value again.This reduced example may seem bad, as the programmer should not do permature close - but lack of the copy/destroy semantics severely limits structuring the code since passing
File
values around easily leads to memory corruption. At this time, the only value safe to assign to other variables isFile()
.I would also recommmend to reconsider the decision for #2; while it is true, that the current implementation is simple, it requires extreme caution when passing around
File
variables. I'd recommend to have full copy+move constructor semantics.If unwanted, I'd recommend to disallow generation by the compiler, so the user is warned:
File(const File&) = delete;
The text was updated successfully, but these errors were encountered: