[libc++] Fix 64-bit file creation for Bionic and Windows

[rprichard]

Summary

Bionic didn't add fopen64 until API 24, but there's no meaningful
distinction between them with Bionic, so just use fopen instead.

On Windows, use _chsize_s instead of _chsize. _chsize uses a 32-bit
long size argument, while _chsize_s uses __int64 instead.

Factor out utils::{off64_t, fopen64, ftruncate64} for use within the
create_file function.

[ldionne]

libcxx/test/support/filesystem_test_helper.h

193–198

I think it would be better to instead add utils::fopen64, utils::ftruncate64 and utils::off64_t. That way, we can simplify this code and hide the complexity inside utils:: above, which was the original goal.

[rprichard]

Factor out utils::{off64_t, fopen64, ftruncate64}.

[rprichard]

On Windows, use int64_t/_chsize_s instead of off_t/_chsize.

I'm not sure if MVS is actually 64-bit or not...

[ldionne]

Thank you. This makes the code simpler while supporting what you need. 🚢 it!

[rprichard]

Adding mstorsjo as subscriber in case they're interested in the Windows part of this change.

[mstorsjo]

This looks ok to me (as long as the configurations in CI still are passing I don't see any issue with it). I don't really see the practical rationale for changing from _chsize to _chsize_s though - if the only difference is that the latter returns the actual error code instead of -1 on error, but we still do return ::_chsize_s(fd, length) ? -1 : 0;, there's not much point in doing that instead of just return ::_chsize(fd, length); right? Or is there some other aspect that isn't spelled out here?

[rprichard]

In D137132#3959285, @mstorsjo wrote:

This looks ok to me (as long as the configurations in CI still are passing I don't see any issue with it). I don't really see the practical rationale for changing from _chsize to _chsize_s though - if the only difference is that the latter returns the actual error code instead of -1 on error, but we still do return ::_chsize_s(fd, length) ? -1 : 0;, there's not much point in doing that instead of just return ::_chsize(fd, length); right? Or is there some other aspect that isn't spelled out here?

_chsize uses a long size parameter, which is 32 bits on Windows, while _chsize_s uses __int64. Maybe I should add that to the commit comment.

[mstorsjo]

In D137132#3964968, @rprichard wrote:

In D137132#3959285, @mstorsjo wrote:

This looks ok to me (as long as the configurations in CI still are passing I don't see any issue with it). I don't really see the practical rationale for changing from _chsize to _chsize_s though - if the only difference is that the latter returns the actual error code instead of -1 on error, but we still do return ::_chsize_s(fd, length) ? -1 : 0;, there's not much point in doing that instead of just return ::_chsize(fd, length); right? Or is there some other aspect that isn't spelled out here?

_chsize uses a long size parameter, which is 32 bits on Windows, while _chsize_s uses __int64. Maybe I should add that to the commit comment.

Oh, I see - thanks. That sounds reasonable yes.

Thanks for updating the commit message!

The patch is ok with me as long as CI keeps working (and the only failure there seems to be a fluke).

libcxx/test/support/filesystem_test_helper.h

#endif
     // off_t). On a 32-bit system this allows us to create a file larger than
     // 2GB.
     std::string create_file(fs::path filename_path, uintmax_t size = 0) {
         std::string filename = filename_path.string();
 #if defined(__LP64__) || defined(_WIN32) || defined(__MVS__)
         auto large_file_fopen = fopen;
         auto large_file_ftruncate = utils::ftruncate;
         using large_file_offset_t = off_t;
+#elif defined(__BIONIC__)
+        // Bionic does not distinguish between fopen and fopen64, and it has an
+        // ftruncate64 that accepts off64_t.
+        auto large_file_fopen = fopen;
+        auto large_file_ftruncate = ftruncate64;
+        using large_file_offset_t = off64_t;
 #else
         auto large_file_fopen = fopen64;
         auto large_file_ftruncate = ftruncate64;
         using large_file_offset_t = off64_t;
 #endif
 
         filename = sanitize_path(std::move(filename));