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

What do these four lines of code do? #1187

Open
cslayers opened this issue May 7, 2024 · 1 comment
Open

What do these four lines of code do? #1187

cslayers opened this issue May 7, 2024 · 1 comment

Comments

@cslayers
Copy link

cslayers commented May 7, 2024

Status Env::RemoveDir(const std::string& dirname) { return DeleteDir(dirname); }

Status Env::DeleteDir(const std::string& dirname) { return RemoveDir(dirname); }

Status Env::RemoveFile(const std::string& fname) { return DeleteFile(fname); }

Status Env::DeleteFile(const std::string& fname) { return RemoveFile(fname); }

`Status Env::RemoveDir(const std::string& dirname) { return DeleteDir(dirname); }
Status Env::DeleteDir(const std::string& dirname) { return RemoveDir(dirname); }

Status Env::RemoveFile(const std::string& fname) { return DeleteFile(fname); }
Status Env::DeleteFile(const std::string& fname) { return RemoveFile(fname); }`

i think calling them will cause stack overflow

@fmorgner
Copy link

fmorgner commented Jun 1, 2024

In short:

I would argue that this would only ever be a problem if you were to call those functions on an instance of leveldb::Env or a derived class which does not adhere to the documented "contract" (in quotes since we don't have contracts in the language yet). However, leveldb::Env is a pure virtual class and thus cannot be instantiated directly, meaning you should never get an instance of that class. In the case of using a derived type that does not adhere to the "contract", the problem you have identified would occur. This would also imply, that whoever implemented that derived class, did so without checking what the expectations towards such a type would be, thus shipping defective code.

In detail:

If you look at the relevant header (include/leveldb/env.h), it points out the following:

Env::DeleteFile is deprecated and only retained to support legacy code. The default implementation calls Env::RemoveFile (as you pointed out above). This is done so that legacy usage of leveldb::Env (e.g. calling Env::DeleteFile) is compatible with modern implementations. Env::RemoveFile calls Env::DeleteFile so that modern usage of Env (e.g. calling Env::RemoveFile) will work with legacy implementations. Note that both functions are functions are virtual, meaning they can be overridden.

Since leveldb::Env is pure virtual, you will need to use an implementation that is derived from it, instead of using leveldb::Env directly. In the basic use case (using what the implementation provides), you will get an instance of such a type by calling the static member function leveldb::Env::Default, which hands you a pointer to an object of a type derived from leveldb::Env. The actual type of the pointed-to objects depends on the operating system you are compiling for. There are currently two implementations, one for Windows and one for POSIX(-ish) operating systems. The decision, as to which implementation is used, is made in the build system configuration. Both of these implementations correctly override leveldb::Env::RemoveFile. You will therefore not see any stack overflow when using the default implementation provided by leveldb.

If you want to provide your own implementation of leveldb::Env, you are required to take care and override leveldb::RemoveFile with an appropriate implementation. If you don't do so, you will see the overflow. However, your implementation would thereby also violate the "contract" of leveldb::Env. If your implementation does violate this "contract", all bets are off as to correctness of your program.

The same argument also holds for leveldb::Env::DeleteDir and leveldb::Env::RemoveDir.

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

2 participants