[llvm] r265068 - Add disk_space() to llvm::fs

[Rui Ueyama via llvm-commits]

On Thu, Mar 31, 2016 at 4:18 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:

>
> On Mar 31, 2016, at 4:14 PM, Rui Ueyama <ruiu at google.com> wrote:
>
> On Thu, Mar 31, 2016 at 4:05 PM, Mehdi Amini via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: mehdi_amini
>> Date: Thu Mar 31 18:05:26 2016
>> New Revision: 265068
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=265068&view=rev
>> Log:
>> Add disk_space() to llvm::fs
>>
>> Summary: Adapted from Boost::filesystem.
>> (This is a reapply by reverting commit r265062 and fixing the WinAPI part)
>>
>> Differential Revision: http://reviews.llvm.org/D18467
>>
>> From: Mehdi Amini <mehdi.amini at apple.com>
>>
>> Modified:
>>     llvm/trunk/include/llvm/Support/FileSystem.h
>>     llvm/trunk/lib/Support/Unix/Path.inc
>>     llvm/trunk/lib/Support/Windows/Path.inc
>>
>> Modified: llvm/trunk/include/llvm/Support/FileSystem.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=265068&r1=265067&r2=265068&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Support/FileSystem.h (original)
>> +++ llvm/trunk/include/llvm/Support/FileSystem.h Thu Mar 31 18:05:26 2016
>> @@ -32,6 +32,7 @@
>>  #include "llvm/ADT/Twine.h"
>>  #include "llvm/Support/DataTypes.h"
>>  #include "llvm/Support/ErrorHandling.h"
>> +#include "llvm/Support/ErrorOr.h"
>>  #include "llvm/Support/TimeValue.h"
>>  #include <ctime>
>>  #include <iterator>
>> @@ -648,6 +649,17 @@ std::error_code identify_magic(const Twi
>>
>>  std::error_code getUniqueID(const Twine Path, UniqueID &Result);
>>
>> +/// @brief Get disk space usage information.
>> +///
>> +/// Note: Users must be careful about "Time Of Check, Time Of Use" kind
>> of bug.
>> +/// Note: Windows reports results according to the quota allocated to
>> the user.
>> +///
>> +/// @param Path Input path.
>> +/// @returns a space_info structure filled with the capacity, free, and
>> +/// available space on the device \a Path is on. A platform specific
>> error_code
>> +/// is returned on error.
>> +ErrorOr<space_info> disk_space(const Twine Path);
>>
>
> I think you need to change the parameter type to `const Twine &`.
>
>
> Oh your right, I was about to do it after your comment on phab, but then
> the windows bot yelled at me and I've been busy fixing the build to the
> point where I forgot. I'm glad you double-checked!
>
> As documented in Twine.h, Twine's implementation relies on the ability to
> store pointers to temporary stock objects which may be deallocated at the
> end of a statement, so having a Twine as a local variable (or a parameter)
> is not correct.
>
>
> You can't take ownership of the Twine, but this is not what happens here,
> is there a correctness issue (I see the inefficiency issue)? The twine will
> points to objects that are in the caller frame and won't be deallocated
> before the end of the call?
>

I read the comment as there is a correctness issue, but I cannot point out
the code which is not safe to use this way, so the comment may be wrong.


> --
> Mehdi
>
>
>
>
>
>> +
>>  /// This class represents a memory mapped file. It is based on
>>  /// boost::iostreams::mapped_file.
>>  class mapped_file_region {
>>
>> Modified: llvm/trunk/lib/Support/Unix/Path.inc
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Path.inc?rev=265068&r1=265067&r2=265068&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Support/Unix/Path.inc (original)
>> +++ llvm/trunk/lib/Support/Unix/Path.inc Thu Mar 31 18:05:26 2016
>> @@ -60,6 +60,24 @@
>>  # define PATH_MAX 4096
>>  #endif
>>
>> +#include <sys/types.h>
>> +#if !defined(__APPLE__) && !defined(__OpenBSD__) && !defined(__ANDROID__)
>> +#include <sys/statvfs.h>
>> +#define STATVFS statvfs
>> +#define STATVFS_F_FRSIZE(vfs) vfs.f_frsize
>> +#else
>> +#ifdef __OpenBSD__
>> +#include <sys/param.h>
>> +#elif defined(__ANDROID__)
>> +#include <sys/vfs.h>
>> +#else
>> +#include <sys/mount.h>
>> +#endif
>> +#define STATVFS statfs
>> +#define STATVFS_F_FRSIZE(vfs) static_cast<uint64_t>(vfs.f_bsize)
>> +#endif
>> +
>> +
>>  using namespace llvm;
>>
>>  namespace llvm {
>> @@ -70,7 +88,7 @@ namespace fs {
>>      defined(__linux__) || defined(__CYGWIN__) || defined(__DragonFly__)
>>  static int
>>  test_dir(char ret[PATH_MAX], const char *dir, const char *bin)
>> -{
>> +{
>>    struct stat sb;
>>    char fullpath[PATH_MAX];
>>
>> @@ -190,6 +208,18 @@ UniqueID file_status::getUniqueID() cons
>>    return UniqueID(fs_st_dev, fs_st_ino);
>>  }
>>
>> +ErrorOr<space_info> disk_space(const Twine Path) {
>> +  struct STATVFS Vfs;
>> +  if (::STATVFS(Path.str().c_str(), &Vfs))
>> +    return std::error_code(errno, std::generic_category());
>> +  auto FrSize = STATVFS_F_FRSIZE(Vfs);
>> +  space_info SpaceInfo;
>> +  SpaceInfo.capacity = static_cast<uint64_t>(Vfs.f_blocks) * FrSize;
>> +  SpaceInfo.free = static_cast<uint64_t>(Vfs.f_bfree) * FrSize;
>> +  SpaceInfo.available = static_cast<uint64_t>(Vfs.f_bavail) * FrSize;
>> +  return SpaceInfo;
>> +}
>> +
>>  std::error_code current_path(SmallVectorImpl<char> &result) {
>>    result.clear();
>>
>>
>> Modified: llvm/trunk/lib/Support/Windows/Path.inc
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=265068&r1=265067&r2=265068&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Support/Windows/Path.inc (original)
>> +++ llvm/trunk/lib/Support/Windows/Path.inc Thu Mar 31 18:05:26 2016
>> @@ -151,6 +151,19 @@ UniqueID file_status::getUniqueID() cons
>>    return UniqueID(VolumeSerialNumber, FileID);
>>  }
>>
>> +ErrorOr<space_info> disk_space(const Twine Path) {
>> +  PULARGE_INTEGER Avail, Total, Free;
>> +  if (!::GetDiskFreeSpaceExA(Path.str().c_str(), &Avail, &Total, &Free))
>> +    return mapWindowsError(::GetLastError());
>> +  space_info SpaceInfo;
>> +  SpaceInfo.capacity =
>> +      (static_cast<uint64_t>(Total.HighPart) << 32) + Total.LowPart;
>> +  SpaceInfo.Free = (static_cast<uint64_t>(Free.HighPart) << 32) +
>> Free.LowPart;
>> +  SpaceInfo.available =
>> +      (static_cast<uint64_t>(Avail.HighPart) << 32) + Avail.LowPart;
>> +  return SpaceInfo;
>> +}
>> +
>>  TimeValue file_status::getLastAccessedTime() const {
>>    ULARGE_INTEGER UI;
>>    UI.LowPart = LastAccessedTimeLow;
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160331/1f57c81c/attachment.html>