Skip to content

Windows: compare file identity in UTIL_isSameFile - #4884

Open
ck0i wants to merge 1 commit into
meta:devfrom
ck0i:win-same-file-identity
Open

ck0i wants to merge 1 commit into
meta:devfrom
ck0i:win-same-file-identity

Conversation

@ck0i

@ck0i ck0i commented Nov 2, 2026

Copy link
Copy Markdown

What and why

On Windows, UTIL_isSameFile() and UTIL_isSameFileStat() compared the two paths with strcmp. Any other spelling of the input passed to -o (different case, a ./ or .\ prefix, an absolute path, dir\..\file, a hard link) got past the check, and FIO_openDstFile() then truncated the input when opening the output. With a 200 byte input, zstd -f a.bin -o A.bin exited 0 and left a.bin as a 13 byte frame of empty content. Fixes #4852.

The Windows branch now opens both paths with FILE_READ_ATTRIBUTES and compares the volume serial number and file index from GetFileInformationByHandle(), the same idea as the st_dev/st_ino check on POSIX. An exact name match still returns early, and a path that cannot be opened counts as a different file. The source vs dictionary check goes through UTIL_isSameFileStat() and picks up the same behavior. playTests.sh gets a -o ./tmp case on all platforms and a -o TMP case on Windows. Both fail with the current dev build under MSVC and pass with this change, and the full playTests.sh passes on Windows with an MSVC x64 build.

Checklist

  • I searched open pull requests and issues: this problem is not already being addressed.
  • Bug fix: it comes with a reproducer or a test that fails without the fix.
  • Performance change: numbers from repeated runs are included, with CPU, compiler, flags and data.
  • I stated below whether AI or other automated tools were used.

Tools used

This PR was filed by an automated agent.

The Windows branch compared names with strcmp, so an output path that
named the input with a different case, a ./ prefix or an absolute path
was not detected and the input was truncated when the output was opened.
Compare the volume serial number and file index instead.

Fixes meta#4852
@meta-cla

meta-cla Bot commented Nov 2, 2026

Copy link
Copy Markdown

Hi @ck0i!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

Windows: zstd -f in -o IN (or -o .\in) truncates the input to 0 bytes: same-file check is a plain strcmp

1 participant