[PATCH] D39464: Define fs::allocate_file which preallocates disk blocks.

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 9 14:36:14 PST 2017


Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:

> Index: llvm/unittests/Support/Path.cpp
> ===================================================================
> --- llvm/unittests/Support/Path.cpp
> +++ llvm/unittests/Support/Path.cpp
> @@ -966,14 +966,16 @@
>  }
>  #endif
>  
> -TEST_F(FileSystemTest, Resize) {
> +TEST_F(FileSystemTest, Allocate) {
>    int FD;
>    SmallString<64> TempPath;
>    ASSERT_NO_ERROR(fs::createTemporaryFile("prefix", "temp", FD, TempPath));
> -  ASSERT_NO_ERROR(fs::resize_file(FD, 123));
> -  fs::file_status Status;
> -  ASSERT_NO_ERROR(fs::status(FD, Status));
> -  ASSERT_EQ(Status.getSize(), 123U);
> +  std::error_code EC = fs::allocate_file(FD, 123);
> +  if (!EC) {
> +    fs::file_status Status;
> +    ASSERT_NO_ERROR(fs::status(FD, Status));
> +    ASSERT_EQ(Status.getSize(), 123U);
> +  }

Why swich away from ASSERT_NO_ERROR?

> -std::error_code resize_file(int FD, uint64_t Size) {
> -#if defined(HAVE_POSIX_FALLOCATE)
> -  // If we have posix_fallocate use it. Unlike ftruncate it always allocates
> -  // space, so we get an error if the disk is full.
> +// Preallocate disk blocks.
> +std::error_code allocate_file(int FD, uint64_t Size) {
> +#ifdef HAVE_FALLOCATE
> +  if (fallocate(FD, 0, 0, Size) == -1)
> +    return std::error_code(errno, std::generic_category());
> +  return std::error_code();
> +#endif

Add a comment about the glibc implementation of posix_fallocate being
the reason we try fallocate first.

LGTM with nits.


More information about the llvm-commits mailing list