Zig: Zig fmt creates backup file when using full path

Created on 2 Mar 2020  路  5Comments  路  Source: ziglang/zig

On Windows using 0.5.0+6a0927d8c

Whenever I use zig fmt with a full path name it makes a file with a name like tCO3sAUiRk2cKtTK.
I'm not entirely sure when it does and when it doesn't create the file. Also sometimes it's empty, but sometimes it contains the old contents?
Maybe it's empty if the file is smaller than 4k?

It seems to be created consistently when I run this powershell code:

ForEach ($item in (Get-ChildItem -Recurse *.zig)) {
    zig fmt $item.FullName;
}
bug contributor friendly os-windows standard library

Most helpful comment

After some janky testing, I think i tracked down the problem:

  • In this working test case that calls cwd().deleteFile, it ends up passing NtCreateFile an ObjectAttributes with RootDirectory as cwd() and ObjectName as the path to the file relative to RootDirectory.
  • In the failing case, RootDirectory is the same, but ObjectName is a fully-qualified path (i.e. \??\C:\Users\Ryan\Programming\Zig\tmp\ThhGb6M3kmMehB_E). This combination is not allowed as far as I can tell

From the NtCreateFile docs:

There are two alternate ways to specify the name of the file to be created or opened with NtCreateFile:

  • As a fully qualified pathname, supplied in the ObjectName member of the input ObjectAttributes
  • As a pathname relative to the directory file represented by the handle in the RootDirectory member of the input ObjectAttributes

Changing RootDirectory to null fixes the fully-qualified case (but obviously breaks non-fully-qualified cases).

Not exactly sure what the fix should be to handle this in the general case, though (don't set RootDirectory if we detect that the path is fully-qualified? expect sub_path_w to always be relative to dirfd and enforce that?).

All 5 comments

Ran into this when using fmt on save via the Sublime Text Zig package as well.

More info:

  • This only happens when no changes are made to the file by zig fmt
  • This means that this if branch is being skipped:

https://github.com/ziglang/zig/blob/00be934569d25e3b041091ff63a4cf6c456d1403/src-self-hosted/stage2.zig#L370-L374

which means finish is not being called on baf, so AtomicFile.deinit (via the deferred BufferedAtomicFile.destroy) is what is expected to delete the temporary file, but that's not happening.

Changing this catch {} to catch unreachable somehow didn't fail but changing it to

cwd().deleteFileC(@ptrCast([*:0]u8, &self.tmp_path_buf)) catch |e| {
    std.debug.warn("error deleting file: {}\n", .{e});
};

prints error deleting file: error.Unexpected

EDIT: the error is coming from std.os.unlinkatW and the value of rc is 0xC000000D (STATUS_INVALID_PARAMETER)

After some janky testing, I think i tracked down the problem:

  • In this working test case that calls cwd().deleteFile, it ends up passing NtCreateFile an ObjectAttributes with RootDirectory as cwd() and ObjectName as the path to the file relative to RootDirectory.
  • In the failing case, RootDirectory is the same, but ObjectName is a fully-qualified path (i.e. \??\C:\Users\Ryan\Programming\Zig\tmp\ThhGb6M3kmMehB_E). This combination is not allowed as far as I can tell

From the NtCreateFile docs:

There are two alternate ways to specify the name of the file to be created or opened with NtCreateFile:

  • As a fully qualified pathname, supplied in the ObjectName member of the input ObjectAttributes
  • As a pathname relative to the directory file represented by the handle in the RootDirectory member of the input ObjectAttributes

Changing RootDirectory to null fixes the fully-qualified case (but obviously breaks non-fully-qualified cases).

Not exactly sure what the fix should be to handle this in the general case, though (don't set RootDirectory if we detect that the path is fully-qualified? expect sub_path_w to always be relative to dirfd and enforce that?).

Thanks for looking into this!

(don't set RootDirectory if we detect that the path is fully-qualified? expect sub_path_w to always be relative to dirfd and enforce that?).

Personally I would prefer the former. My reasoning is that opening a file is a lot like typing a path in a shell, from a certain working directory. You would expect to be able to specify an absolute path, or a path relative to the open directory handle.

Cool, thanks for fixing it!

Was this page helpful?
0 / 5 - 0 ratings