Windows rename_internal function incorrectly passing character count instead of byte count to SetFileInformationByHandle.

[• benhillis]

Summary

The rename_internal function used for Windows has a minor bug where the filename length is passed as a character count instead of a byte count. Windows internally ignores this field, but other tools that hook NT api's may use the documented behavior:

MSDN documentation specifying the size should be in bytes:
https://docs.microsoft.com/en-us/windows/desktop/api/winbase/ns-winbase-_file_rename_info

[• benhillis]

Added a few reviewers based on git blame, please feel free to forward to anybody who would be more appropriate.

[smeenai]

This LGTM, but before I approve, I'd like to look around the surrounding code to confirm things are how I expect, but this patch is uploaded without context :( Could you reupload with context? See https://llvm.org/docs/Phabricator.html (the part about using -U999999 in your git commands to generate full context).

[• benhillis]

Ah, my mistake. I will update the patch.

[smeenai]

That's the intent ... it uploads the entire file to Phabricator, so that it can be viewed through the web interface. The arcanist tool does that for you automatically, as an alternative to have to copy and paste a giant block of text around.

In this case, I just looked at the surrounding context myself, and this LGTM. Do you need someone to commit this? Context will be helpful for future patches though.

[• benhillis]

@smeenai - Thanks for the explanation, I misunderstood what the -U999999 was doing. Yes if this patch is accepted, I'll need somebody to commit it for me to the LLVM mainline.

[smeenai]

Committed in r348995. (I also took the liberty of adding a [Support] tag to the title and wrapping the commit message, in line with LLVM conventions; see https://llvm.org/docs/DeveloperPolicy.html#commit-messages). Thanks for the patch!

llvm/trunk/lib/Support/Windows/Path.inc

if (auto EC = widenPath(To, ToWide))
     return EC;
 
   std::vector<char> RenameInfoBuf(sizeof(FILE_RENAME_INFO) - sizeof(wchar_t) +
                                   (ToWide.size() * sizeof(wchar_t)));
   FILE_RENAME_INFO &RenameInfo =
       *reinterpret_cast<FILE_RENAME_INFO *>(RenameInfoBuf.data());
   RenameInfo.ReplaceIfExists = ReplaceIfExists;
   RenameInfo.RootDirectory = 0;
-  RenameInfo.FileNameLength = ToWide.size();
+  RenameInfo.FileNameLength = ToWide.size() * sizeof(wchar_t);
   std::copy(ToWide.begin(), ToWide.end(), &RenameInfo.FileName[0]);
 
   SetLastError(ERROR_SUCCESS);
   if (!SetFileInformationByHandle(FromHandle, FileRenameInfo, &RenameInfo,
                                   RenameInfoBuf.size())) {
     unsigned Error = GetLastError();
     if (Error == ERROR_SUCCESS)
       Error = ERROR_CALL_NOT_IMPLEMENTED; // Wine doesn't always set error code.