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

Misleading variable names in WriteFile #18

Closed
brettkettering opened this issue Aug 21, 2012 · 1 comment
Closed

Misleading variable names in WriteFile #18

brettkettering opened this issue Aug 21, 2012 · 1 comment

Comments

@brettkettering
Copy link
Contributor

4/20/2011:
WriteFile? has a variable called physical path. However, there is a time when WriteFile? is called from fuse:f_rename() when the path is reset to be a logical path. This should be cleaned up (I don't know just how off the top of my head) so that's it's not misleading.

[There was a similar problem with Index where it was labeling a variable as logical_path but it was always exclusively holding a physical path so that was easily fixed by just renaming the Index::logical_path variable to Index::physical_path.]

Below is the email from Chuck discovering this problem. Thanks Chuck!

The leak described in his email has been patched as well in the trunk.

--- Begin forwarded message from Chuck Cranor ---
From: Chuck Cranor <chuck@…>
To: PLFS Development <plfs-devel@…>
Date: Wed, 20 Apr 2011 12:36:31 -0600
Subject: [Plfs-devel] leak + odd

leak?
Container::hostdir_index_read() calls opendir(), never calls closedir().

if((dirp=opendir(path)) == NULL) {

odd?

OpenFile?.cpp says:
void Plfs_fd::setPath( string p ) {
this->path = p;
if ( writefile ) writefile->setPath( p );
if ( index ) index->setPath( p );
}

WriteFile?.cpp says:
void WriteFile::setPath ( string p ) {
this->physical_path = p;
this->has_been_renamed = true;
}

Index.cpp says:
void Index::setPath( string p ) {
this->logical_path = p;
}

so is path a "physical_path" as WriteFile?.cpp says, or is it a "logical_path" like Index.cpp says?

looking at it, setPath() functions only get called in 2 places:

  • f_rename in FUSE calls Plfs_fd::setPath()
  • plfs_parindex_read() calls Index::setPath()
    hmm.

chuck

@brettkettering
Copy link
Contributor Author

This can be closed.

The merge from EMC engineering that added a LogicalFD class and moved all the code into ContainerFD and added FlatFileFD cleaned all this up. Now only the expandPath code ever sees a logical path. All the internal container and flatfile code and index code (which is part of container) see only physical paths.

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

No branches or pull requests

1 participant