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

os: SameFile should not only check inodes and devices #36895

Closed
Helflym opened this issue Jan 30, 2020 · 2 comments
Closed

os: SameFile should not only check inodes and devices #36895

Helflym opened this issue Jan 30, 2020 · 2 comments

Comments

@Helflym
Copy link
Contributor

@Helflym Helflym commented Jan 30, 2020

What version of Go are you using (go version)?

tip

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

Every Unix systems (tested on AIX and Linux).

What did you do?

I'm calling os.SameFile to check if a file have been moved, using the two FileInfos associated with the old path and the new path.
However, there is some case where the device and the inode are the same.

I don't have a pure Go example, as it isn't 100% reproducible. But, here is a bash version.

$ touch alpha && ls -i alpha
15863201 alpha
$ rm alpha
$ mkdir beta && ls -i
15863201 beta

As you can see, both files end up with the same inode. Therefore, os.SameFile on Unix systems will return true, even if it's one is a basic file and one a directory.

I think it's acceptable when two basic files are involved (it can be considered as a "move" and a "write" afterall), same for two directories. But for the shown case, it seems weird to me. I don't know if there is a way to transform a file to a directory directly.

Shouldn't os.SameFile also check IsDir() ?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 30, 2020

Adding an IsDir check won't fix the problem. If you call os.Stat on a file, and then remove that file, it's no longer meaningful to use os.SameFile with that os.FileInfo data. You can create a file at inode number N, remove that file, and then create another file at inode number N. They can be completely different files. os.SameFile only returns valid results for files that still exist. If you need to know whether a file has been moved, you need to hash the contents of the file.

@Helflym

This comment has been minimized.

Copy link
Contributor Author

@Helflym Helflym commented Jan 31, 2020

Indeed, I've thought about the fact that os.FileInfo isn't accurate once a file have been deleted. It still good to have this issue with anyone comes across a similar problem.
I'll check if I can hash the file's content without impacting performances too much.
Thanks @ianlancetaylor

@Helflym Helflym closed this Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.