<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Mar 31, 2016 at 4:18 PM, Mehdi Amini <span dir="ltr"><<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div class="h5"><blockquote type="cite"><div>On Mar 31, 2016, at 4:14 PM, Rui Ueyama <<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>> wrote:</div><br><div><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_extra"><div class="gmail_quote">On Thu, Mar 31, 2016 at 4:05 PM, Mehdi Amini via llvm-commits<span> </span><span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: mehdi_amini<br>Date: Thu Mar 31 18:05:26 2016<br>New Revision: 265068<br><br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=265068&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=265068&view=rev</a><br>Log:<br>Add disk_space() to llvm::fs<br><br>Summary: Adapted from Boost::filesystem.<br>(This is a reapply by reverting commit r265062 and fixing the WinAPI part)<br><br>Differential Revision:<span> </span><a href="http://reviews.llvm.org/D18467" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18467</a><br><br>From: Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>><br><br>Modified:<br>   <span> </span>llvm/trunk/include/llvm/Support/FileSystem.h<br>   <span> </span>llvm/trunk/lib/Support/Unix/Path.inc<br>   <span> </span>llvm/trunk/lib/Support/Windows/Path.inc<br><br>Modified: llvm/trunk/include/llvm/Support/FileSystem.h<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=265068&r1=265067&r2=265068&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=265068&r1=265067&r2=265068&view=diff</a><br>==============================================================================<br>--- llvm/trunk/include/llvm/Support/FileSystem.h (original)<br>+++ llvm/trunk/include/llvm/Support/FileSystem.h Thu Mar 31 18:05:26 2016<br>@@ -32,6 +32,7 @@<br> #include "llvm/ADT/Twine.h"<br> #include "llvm/Support/DataTypes.h"<br> #include "llvm/Support/ErrorHandling.h"<br>+#include "llvm/Support/ErrorOr.h"<br> #include "llvm/Support/TimeValue.h"<br> #include <ctime><br> #include <iterator><br>@@ -648,6 +649,17 @@ std::error_code identify_magic(const Twi<br><br> std::error_code getUniqueID(const Twine Path, UniqueID &Result);<br><br>+/// @brief Get disk space usage information.<br>+///<br>+/// Note: Users must be careful about "Time Of Check, Time Of Use" kind of bug.<br>+/// Note: Windows reports results according to the quota allocated to the user.<br>+///<br>+/// @param Path Input path.<br>+/// @returns a space_info structure filled with the capacity, free, and<br>+/// available space on the device \a Path is on. A platform specific error_code<br>+/// is returned on error.<br>+ErrorOr<space_info> disk_space(const Twine Path);<br></blockquote><div><br></div><div>I think you need to change the parameter type to `const Twine &`. </div></div></div></div></div></blockquote><div><br></div></div></div><div>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!</div><span class=""><br><blockquote type="cite"><div><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_extra"><div class="gmail_quote"><div>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.</div></div></div></div></div></blockquote><div><br></div></span><div>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?</div></div></div></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class="HOEnZb"><font color="#888888"><div>-- </div><div>Mehdi</div></font></span><div><div class="h5"><div><br></div><div><br></div><br><blockquote type="cite"><div><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">+<br> /// This class represents a memory mapped file. It is based on<br> /// boost::iostreams::mapped_file.<br> class mapped_file_region {<br><br>Modified: llvm/trunk/lib/Support/Unix/Path.inc<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Path.inc?rev=265068&r1=265067&r2=265068&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Path.inc?rev=265068&r1=265067&r2=265068&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Support/Unix/Path.inc (original)<br>+++ llvm/trunk/lib/Support/Unix/Path.inc Thu Mar 31 18:05:26 2016<br>@@ -60,6 +60,24 @@<br> # define PATH_MAX 4096<br> #endif<br><br>+#include <sys/types.h><br>+#if !defined(__APPLE__) && !defined(__OpenBSD__) && !defined(__ANDROID__)<br>+#include <sys/statvfs.h><br>+#define STATVFS statvfs<br>+#define STATVFS_F_FRSIZE(vfs) vfs.f_frsize<br>+#else<br>+#ifdef __OpenBSD__<br>+#include <sys/param.h><br>+#elif defined(__ANDROID__)<br>+#include <sys/vfs.h><br>+#else<br>+#include <sys/mount.h><br>+#endif<br>+#define STATVFS statfs<br>+#define STATVFS_F_FRSIZE(vfs) static_cast<uint64_t>(vfs.f_bsize)<br>+#endif<br>+<br>+<br> using namespace llvm;<br><br> namespace llvm {<br>@@ -70,7 +88,7 @@ namespace fs {<br>     defined(__linux__) || defined(__CYGWIN__) || defined(__DragonFly__)<br> static int<br> test_dir(char ret[PATH_MAX], const char *dir, const char *bin)<br>-{<br>+{<br>   struct stat sb;<br>   char fullpath[PATH_MAX];<br><br>@@ -190,6 +208,18 @@ UniqueID file_status::getUniqueID() cons<br>   return UniqueID(fs_st_dev, fs_st_ino);<br> }<br><br>+ErrorOr<space_info> disk_space(const Twine Path) {<br>+  struct STATVFS Vfs;<br>+  if (::STATVFS(Path.str().c_str(), &Vfs))<br>+    return std::error_code(errno, std::generic_category());<br>+  auto FrSize = STATVFS_F_FRSIZE(Vfs);<br>+  space_info SpaceInfo;<br>+  SpaceInfo.capacity = static_cast<uint64_t>(Vfs.f_blocks) * FrSize;<br>+  SpaceInfo.free = static_cast<uint64_t>(Vfs.f_bfree) * FrSize;<br>+  SpaceInfo.available = static_cast<uint64_t>(Vfs.f_bavail) * FrSize;<br>+  return SpaceInfo;<br>+}<br>+<br> std::error_code current_path(SmallVectorImpl<char> &result) {<br>   result.clear();<br><br><br>Modified: llvm/trunk/lib/Support/Windows/Path.inc<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=265068&r1=265067&r2=265068&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=265068&r1=265067&r2=265068&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Support/Windows/Path.inc (original)<br>+++ llvm/trunk/lib/Support/Windows/Path.inc Thu Mar 31 18:05:26 2016<br>@@ -151,6 +151,19 @@ UniqueID file_status::getUniqueID() cons<br>   return UniqueID(VolumeSerialNumber, FileID);<br> }<br><br>+ErrorOr<space_info> disk_space(const Twine Path) {<br>+  PULARGE_INTEGER Avail, Total, Free;<br>+  if (!::GetDiskFreeSpaceExA(Path.str().c_str(), &Avail, &Total, &Free))<br>+    return mapWindowsError(::GetLastError());<br>+  space_info SpaceInfo;<br>+  SpaceInfo.capacity =<br>+      (static_cast<uint64_t>(Total.HighPart) << 32) + Total.LowPart;<br>+  SpaceInfo.Free = (static_cast<uint64_t>(Free.HighPart) << 32) + Free.LowPart;<br>+  SpaceInfo.available =<br>+      (static_cast<uint64_t>(Avail.HighPart) << 32) + Avail.LowPart;<br>+  return SpaceInfo;<br>+}<br>+<br> TimeValue file_status::getLastAccessedTime() const {<br>   ULARGE_INTEGER UI;<br>   UI.LowPart = LastAccessedTimeLow;<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></blockquote></div></div></div></div></blockquote></div></div></div><br></div></blockquote></div><br></div></div>