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

File lacks a copy constructor #66

Open
svatoun opened this issue Aug 4, 2019 · 3 comments
Open

File lacks a copy constructor #66

svatoun opened this issue Aug 4, 2019 · 3 comments
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project

Comments

@svatoun
Copy link

svatoun commented Aug 4, 2019

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:

void doSomething(File f) {
   // does something nice.
   f.close();
}

void func() {
   File myFile = ....; // obtain a file somehow
   doSomething(myFile);
   myFile.close();
}

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;

@thekunalsaini
Copy link

thekunalsaini commented Feb 24, 2020

Consider this Code:

Class name(argument) { File f = value ; // intialized value according to requirement };
Copy constructor is to made in class then ,we easily pass by value.

@thekunalsaini

This comment was marked as spam.

@drewfish
Copy link

drewfish commented Dec 5, 2020

Could your function use a reference instead? void doSomething(File &f)

@per1234 per1234 added type: imperfection Perceived defect in any part of project topic: code Related to content of the project itself labels Apr 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

No branches or pull requests

4 participants