[PATCH] D38570: Support: Rewrite Windows implementation of sys::fs::rename to be more POSIXy.

[Peter Collingbourne via llvm-commits]

HI Adrian,

If that fix works for you, would you mind checking it in?

Peter

On Mon, Oct 9, 2017 at 10:35 AM, Adrian McCarthy <amccarth at google.com>
wrote:

> I just came across this, too.  I'm testing this fix:
>
> std::copy(ToWide.begin(), ToWide.end(), &RenameInfo.FileName[0]);
>
> which makes the target argument a plain old pointer rather than an array
> of length 1.
>
> On Mon, Oct 9, 2017 at 10:13 AM, George Rimar via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> grimar added inline comments.
>>
>>
>> ================
>> Comment at: llvm/trunk/lib/Support/Windows/Path.inc:375
>> +  RenameInfo.FileNameLength = ToWide.size();
>> +  std::copy(ToWide.begin(), ToWide.end(), RenameInfo.FileName);
>> +
>> ----------------
>> This asserts under windows/debug.
>> `RenameInfo.FileName` here is `WCHAR  FileName[1];` and std::copy checks
>> it's size
>> for overflow and fails:
>>
>> ```
>>         _Myiter& operator+=(difference_type _Off)
>>                 {       // increment by integer
>>  #if _ITERATOR_DEBUG_LEVEL == 2
>>                 if (_Size < _Idx + _Off)
>>                         {       // report error
>>                         _DEBUG_ERROR("array iterator + offset out of
>> range");
>>                         _SCL_SECURE_OUT_OF_RANGE;
>>                         }
>> ```
>>
>> ```
>> template<class _InIt,
>>         class _OutTy,
>>         size_t _OutSize> inline
>>         _OutTy *copy(_InIt _First, _InIt _Last,
>>                 _OutTy (&_Dest)[_OutSize])
>>         {       // copy [_First, _Last) to [_Dest, ...)
>> ```
>>
>> I would suggest to use memcpy instead.
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D38570
>>
>>
>>
>>
>


-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171009/99aac00f/attachment.html>