windows: _chsize_s not available on MinGW #382

Closed
saghul opened this Issue Apr 19, 2012 · 12 comments

Projects

None yet

5 participants

@saghul
Contributor
saghul commented Apr 19, 2012

After this change: d5acfd0#L3R580 libuv won't compile on MinGW because apparently _chsize_s is not available. Apparently _chsize is limited to 32bits offsets so we can't use that in case the _s version is not available.

@bnoordhuis
Contributor

That looks like it at least partially explains #381. I'll see if I can fix it.

@saghul
Contributor
saghul commented Apr 19, 2012

Indeed, that seems to be the case. FWIW, I think MinGW uses the 7.0 CRT.

@luislavena
Contributor

@saghul MinGW uses default MSVCRT which happens to be internally version 7.0 but used to be 6.0 in older versions of Windows.

You can change MinGW to use a different version of CRT but that will force you to build the link libraries for msvcr80 or msvcr90 and also distribute those with you (which you can't)

I did mention @piscisaureus about _s function:
ea50126#commitcomment-1181845

But seems that what was supposed to target only 0.6got merged into master?

@saghul
Contributor
saghul commented Apr 19, 2012

@luislavena Thanks for the info!

AFAIK the '64' variant of those functions is now the default and apparently that call to _chsize_s made it to master. I changed it to _chsize and compilation worked so apparently it's the only occurrence.

@luislavena
Contributor

Can't something similar be achieved with _ftruncate64 ? I believe is present on both MinGW and mingw-w64 builds. Will check that later and confirm.

@luislavena
Contributor

@saghul while mingw-w64 already defines _chsize_s and mingw, what about an approach that uses SetFilePointer and SetEndOfFile?

http://msdn.microsoft.com/en-us/library/windows/desktop/aa365541(v=vs.85).aspx
http://msdn.microsoft.com/en-us/library/windows/desktop/aa365531(v=vs.85).aspx

The only drawback is that instead of a file descriptor we need a HANDLE and split int64_t offset in higher and lower byte.

@piscisaureus what do you think about the approach? Let me know and I can work out a patch to replace fs__ftruncate.

@piscisaureus
Member

There is another issue with _chsize and SetEndOfFile, which is that it requires moving the file pointer. That's not compatible with ftruncate.

I think you should do it with NtSetInformationFile and the FileEndOfFileInformation command. See http://msdn.microsoft.com/en-us/library/windows/hardware/ff557671%28v=vs.85%29.aspx and http://msdn.microsoft.com/en-us/library/windows/hardware/ff545780%28v=vs.85%29.aspx

@luislavena
Contributor

@piscisaureus thanks, I'll see if I can work on this by early next week (a bit overloaded with work).

@saghul
Contributor
saghul commented Apr 20, 2012

@luislavena I wish I could help, but this is a bit out of my reach, I have no idea about the Windows CRT... I do have the build chain ready, so at least I could test patches :-)

Thanks guys!

@Keno
Contributor
Keno commented Apr 28, 2012

Ok, I had the same issue and this is how I tried to fix it (the second one). Can somebody have a look at it? If it looks good, I'll open a pull request.

@piscisaureus
Member

Looks like you're almost there. This works but if it doesn't the error is not correctly reported.

You probably want to do something like:

if (status == STATUS_SUCCES) {
  SET_REQ_RESULT(req, 0);
} else {
  SET_REQ_WIN32_ERROR(req, pRtlNtStatusToDosError(status));
}
@Keno Keno added a commit to JuliaLang/libuv that referenced this issue Apr 28, 2012
@Keno Keno Attempt to fix #382 9cec5f1
@Keno Keno referenced this issue Apr 29, 2012
Closed

Fix #382 #397

@Keno
Contributor
Keno commented Apr 29, 2012

Fixed and opened a pull request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment