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
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@Helflym
Copy link
Contributor

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
Copy link
Contributor

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.

@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 30, 2020
@Helflym
Copy link
Contributor Author

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 as completed Jan 31, 2020
@golang golang locked and limited conversation to collaborators Jan 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

3 participants