Introduce FileSystem::CalculateMD5AsString that supports any platform and make existing FileSystem::CalculateMD5 to use it.

[ovyalov]

Summary

Introduce FileSystem::CalculateMD5AsString that supports any platform and make existing FileSystem::CalculateMD5 to use it.
Code from this CL will be used in http://reviews.llvm.org/D7709 to get MD5 string by a file path.

[zturner]

Thanks for working on this. It always bothered me that this was platform-specific.

include/lldb/Host/FileSystem.h

39–40

I think it might be more useful to put these functions in LLVM. I'm thinking we should delete both of these and add

void llvm::sys::fs::md5_contents(const Twine &Path, llvm::MD5Result &Result);

[zturner]

On second though, maybe this can stay here. Most applications MD5 memory buffers, not entire files, because they're the ones writing the files to begin with. I will put some comments in a followup

[zturner]

source/Host/common/FileSystem.cpp

22

I think it would be better if this file contained a function in an anonymous namespace with a signature like this:

bool CalculateMD5(StringRef path, MD5Result &result);

Then, both this function and the other function could call this to get an MD5Result. From there, this whole function becomes:

llvm::MD5::MD5Result result;
CalculateMD5(StringRef(file_spec.GetPath()), result);
high = ((uint64_t*)result)[0];
low = ((uint64_t*)result)[1];

which is much nicer than first converting a number to a string, and then converting two strings back to numbers.

46

I would prefer if we used a vector<char> here. Taking up 4K of stack is a little bit obnoxious for a simple function.

61

Similarly, with the aforementioned change, this function becomes:

llvm::MD5::MD5Result result;
CalculateMD5(StringRef(file_spec.GetPath()), result);
llvm::MD5::stringifyResult(result, str);

[clayborg]

Move the code that calculates the MD5 into a static function that both FileSystem::CalculateMD5AsString() and FileSystem::CalculateMD5() use. See inlined comments for details.

include/lldb/Host/FileSystem.h

40

I would rather not move this to LLVM just yet. We merge the top of tree SVN to many other branches internally here at apple and these internal branches link against older versions of llvm/clang that won't have this new functionality, so please leave this in FileSystem for now.

source/Host/common/FileSystem.cpp

38–58

Move the MD5 calculation into a static function:

bool
CalculateMD5 (const FileSpec &file_spec, llvm::MD5::MD5Result &md5_result);

And have both versions use it. The version that wants to return two uint64 values should be able to cast the llvm::MD5::MD5Result to a "uint64_t *' and access both numbers and it then avoids creating a string and calling strtoull on the two strings.

[ovyalov]

Fixed accordingly to your suggestions.
Please take another look.

Thank you.

[zturner]

source/Host/common/FileSystem.cpp

63

I'm not sure how important it is for you to have the result in an std::string. For example, are you going to pass it in to another API that accepts an std::string? Or are you just going to print it or log it? As a result, you might consider changing the second argument to an llvm::SmallString<32> & which will save a heap allocation and a string copy. But this is minor, so up to you. Otherwise LGTM

[clayborg]

Looks good.

[ovyalov]

source/Host/common/FileSystem.cpp

63

Let me leave it as std::string since I'd prefer to keep this function signature more generic.

[ovyalov]

AFFECTED FILES

/lldb/trunk/include/lldb/Host/FileSystem.h
/lldb/trunk/lldb.xcodeproj/project.pbxproj
/lldb/trunk/source/Host/CMakeLists.txt
/lldb/trunk/source/Host/common/FileSystem.cpp
/lldb/trunk/source/Host/posix/FileSystem.cpp
/lldb/trunk/source/Host/windows/FileSystem.cpp

USERS

ovyalov (Author)

http://reviews.llvm.org/rL230036

include/lldb/Host/FileSystem.h

public:
     static Error SetFilePermissions(const char *path, uint32_t file_permissions);
     static lldb::user_id_t GetFileSize(const FileSpec &file_spec);
     static bool GetFileExists(const FileSpec &file_spec);
 
     static Error Symlink(const char *src, const char *dst);
     static Error Readlink(const char *path, char *buf, size_t buf_len);
     static Error Unlink(const char *path);
 
     static bool CalculateMD5(const FileSpec &file_spec, uint64_t &low, uint64_t &high);
+    static bool CalculateMD5AsString(const FileSpec &file_spec, std::string& digest_str);
 };
 }
 
 #endif

lldb.xcodeproj/project.pbxproj

/* Begin PBXBuildFile section */
     236124A51986B4E2004EFC37 /* Socket.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 236124A31986B4E2004EFC37 /* Socket.cpp */; };
     2377C2F819E613C100737875 /* PipePosix.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2377C2F719E613C100737875 /* PipePosix.cpp */; };
     23CDD8FB19D47B3600461DDC /* ThreadStateCoordinator.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 23CDD8E819D3E13200461DDC /* ThreadStateCoordinator.cpp */; };
     23DDF226196C3EE600BB8417 /* CommandOptionValidators.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 23DDF224196C3EE600BB8417 /* CommandOptionValidators.cpp */; };
     23EDE33319269E7C00F6A132 /* NativeRegisterContext.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 23EDE3301926839700F6A132 /* NativeRegisterContext.cpp */; };
     23EFE389193D1ABC00E54E54 /* SBTypeEnumMember.h in Headers */ = {isa = PBXBuildFile; fileRef = 23EFE388193D1ABC00E54E54 /* SBTypeEnumMember.h */; settings = {ATTRIBUTES = (Public, ); }; };
     23EFE38B193D1AEC00E54E54 /* SBTypeEnumMember.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 23EFE38A193D1AEC00E54E54 /* SBTypeEnumMember.cpp */; };
     23F4034D1926E0F60046DC9B /* NativeRegisterContextRegisterInfo.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 23F403481926CC250046DC9B /* NativeRegisterContextRegisterInfo.cpp */; };
+    250D6AE31A9679440049CC70 /* FileSystem.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 250D6AE11A9679270049CC70 /* FileSystem.cpp */; };
     25420ECD1A6490B8009ADBCB /* OptionValueChar.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 25420ECC1A6490B8009ADBCB /* OptionValueChar.cpp */; };
     25420ED21A649D88009ADBCB /* PipeBase.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 25420ED11A649D88009ADBCB /* PipeBase.cpp */; };
     254FBB951A81AA7F00BD6378 /* SBLaunchInfo.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 254FBB941A81AA7F00BD6378 /* SBLaunchInfo.cpp */; };
     254FBB971A81B03100BD6378 /* SBLaunchInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = 254FBB961A81B03100BD6378 /* SBLaunchInfo.h */; settings = {ATTRIBUTES = (Public, ); }; };
     254FBBA31A9166F100BD6378 /* SBAttachInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = 254FBBA21A9166F100BD6378 /* SBAttachInfo.h */; settings = {ATTRIBUTES = (Public, ); }; };
     254FBBA51A91670E00BD6378 /* SBAttachInfo.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 254FBBA41A91670E00BD6378 /* SBAttachInfo.cpp */; };
     260157C61885F51C00F875CF /* libpanel.dylib in Frameworks */ = {isa = PBXBuildFile; fileRef = 260157C41885F4FF00F875CF /* libpanel.dylib */; };
     260157C81885F53100F875CF /* libpanel.dylib in Frameworks */ = {isa = PBXBuildFile; fileRef = 260157C41885F4FF00F875CF /* libpanel.dylib */; };
/* Begin PBXFileReference section */
     23DDF224196C3EE600BB8417 /* CommandOptionValidators.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = CommandOptionValidators.cpp; path = source/Interpreter/CommandOptionValidators.cpp; sourceTree = "<group>"; };
     23EDE3301926839700F6A132 /* NativeRegisterContext.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; name = NativeRegisterContext.cpp; path = source/Host/common/NativeRegisterContext.cpp; sourceTree = "<group>"; };
     23EDE3311926843600F6A132 /* NativeRegisterContext.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = NativeRegisterContext.h; path = include/lldb/Host/common/NativeRegisterContext.h; sourceTree = "<group>"; };
     23EDE3371926AAD500F6A132 /* RegisterInfoInterface.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = RegisterInfoInterface.h; path = Utility/RegisterInfoInterface.h; sourceTree = "<group>"; };
     23EFE388193D1ABC00E54E54 /* SBTypeEnumMember.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SBTypeEnumMember.h; path = include/lldb/API/SBTypeEnumMember.h; sourceTree = "<group>"; };
     23EFE38A193D1AEC00E54E54 /* SBTypeEnumMember.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; name = SBTypeEnumMember.cpp; path = source/API/SBTypeEnumMember.cpp; sourceTree = "<group>"; };
     23F403471926C8D50046DC9B /* NativeRegisterContextRegisterInfo.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = NativeRegisterContextRegisterInfo.h; path = include/lldb/Host/common/NativeRegisterContextRegisterInfo.h; sourceTree = "<group>"; };
     23F403481926CC250046DC9B /* NativeRegisterContextRegisterInfo.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = NativeRegisterContextRegisterInfo.cpp; path = source/Host/common/NativeRegisterContextRegisterInfo.cpp; sourceTree = "<group>"; };
+    250D6AE11A9679270049CC70 /* FileSystem.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = FileSystem.cpp; sourceTree = "<group>"; };
     25420ECC1A6490B8009ADBCB /* OptionValueChar.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = OptionValueChar.cpp; path = source/Interpreter/OptionValueChar.cpp; sourceTree = "<group>"; };
     25420ECE1A64911B009ADBCB /* OptionValueChar.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = OptionValueChar.h; path = include/lldb/Interpreter/OptionValueChar.h; sourceTree = "<group>"; };
     25420ED11A649D88009ADBCB /* PipeBase.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PipeBase.cpp; sourceTree = "<group>"; };
     254FBB921A81AA5200BD6378 /* SBLaunchInfo.i */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c.preprocessed; path = SBLaunchInfo.i; sourceTree = "<group>"; };
     254FBB941A81AA7F00BD6378 /* SBLaunchInfo.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SBLaunchInfo.cpp; path = source/API/SBLaunchInfo.cpp; sourceTree = "<group>"; };
     254FBB961A81B03100BD6378 /* SBLaunchInfo.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SBLaunchInfo.h; path = include/lldb/API/SBLaunchInfo.h; sourceTree = "<group>"; };
     254FBBA21A9166F100BD6378 /* SBAttachInfo.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SBAttachInfo.h; path = include/lldb/API/SBAttachInfo.h; sourceTree = "<group>"; };
     254FBBA41A91670E00BD6378 /* SBAttachInfo.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SBAttachInfo.cpp; path = source/API/SBAttachInfo.cpp; sourceTree = "<group>"; };
4CEE62F71145F1C70064CF93 /* GDB Remote */ = {
       );
       name = "GDB Remote";
       path = "gdb-remote";
       sourceTree = "<group>";
     };
     69A01E1A1236C5D400C660B5 /* common */ = {
       isa = PBXGroup;
       children = (
+        250D6AE11A9679270049CC70 /* FileSystem.cpp */,
         33E5E8411A672A240024ED68 /* StringConvert.cpp */,
         25420ED11A649D88009ADBCB /* PipeBase.cpp */,
         26CFDCA2186163A4000E63E5 /* Editline.cpp */,
         260C6EA213011581005E16B0 /* File.cpp */,
         26FA43171301048600E71120 /* FileSpec.cpp */,
         69A01E1B1236C5D400C660B5 /* Condition.cpp */,
         69A01E1C1236C5D400C660B5 /* Host.cpp */,
         3FDFE53419A29327009756A7 /* HostInfoBase.cpp */,
2689FFC613353D7A00698AC0 /* Sources */ = {
         8CF02AE919DCC01900B14BE0 /* InstrumentationRuntime.cpp in Sources */,
         2689002B13353E0400698AC0 /* AddressRange.cpp in Sources */,
         236124A41986B4E2004EFC37 /* IOObject.cpp in Sources */,
         2689002C13353E0400698AC0 /* AddressResolver.cpp in Sources */,
         2689002D13353E0400698AC0 /* AddressResolverFileLine.cpp in Sources */,
         2689002E13353E0400698AC0 /* AddressResolverName.cpp in Sources */,
         2689002F13353E0400698AC0 /* ArchSpec.cpp in Sources */,
         2689003013353E0400698AC0 /* Baton.cpp in Sources */,
+        250D6AE31A9679440049CC70 /* FileSystem.cpp in Sources */,
         2689003113353E0400698AC0 /* Broadcaster.cpp in Sources */,
         49DCF702170E70120092F75E /* Materializer.cpp in Sources */,
         2689003213353E0400698AC0 /* Communication.cpp in Sources */,
         2689003313353E0400698AC0 /* Connection.cpp in Sources */,
         2689003513353E0400698AC0 /* ConstString.cpp in Sources */,
         2689003613353E0400698AC0 /* DataBufferHeap.cpp in Sources */,
         2689003713353E0400698AC0 /* DataBufferMemoryMap.cpp in Sources */,
         2689003813353E0400698AC0 /* DataExtractor.cpp in Sources */,

source/Host/CMakeLists.txt

 macro(add_host_subdirectory group)
   list(APPEND HOST_SOURCES ${ARGN})
   source_group(${group} FILES ${ARGN})
 endmacro()
 
 add_host_subdirectory(common
   common/Condition.cpp
   common/File.cpp
   common/FileCache.cpp
   common/FileSpec.cpp
+  common/FileSystem.cpp
   common/Host.cpp
   common/HostInfoBase.cpp
   common/HostNativeThreadBase.cpp
   common/HostProcess.cpp
   common/HostThread.cpp
   common/IOObject.cpp
   common/Mutex.cpp
   common/MonitoringProcessLauncher.cpp

source/Host/common/FileSystem.cpp

+//===-- FileSystem.cpp ------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/FileSystem.h"
+
+#include "llvm/Support/MD5.h"
+
+#include <fstream>
+
+using namespace lldb;
+using namespace lldb_private;
+
+bool
+FileSystem::CalculateMD5(const FileSpec &file_spec, uint64_t &low, uint64_t &high)
+{
+    std::string hash_string;
+    if (!CalculateMD5AsString(file_spec, hash_string))
+        return false;
+
+    if (hash_string.size() != 32)
+        return false;
+
+    std::string part1(hash_string, 0, 16);
+    std::string part2(hash_string, 16);
+    const char *part1_cstr = part1.c_str();
+    const char *part2_cstr = part2.c_str();
+    high = ::strtoull(part1_cstr, NULL, 16);
+    low = ::strtoull(part2_cstr, NULL, 16);
+    return true;
+}
+
+bool
+FileSystem::CalculateMD5AsString(const FileSpec &file_spec, std::string& digest_str)
+{
+    llvm::MD5 md5_hash;
+    std::ifstream file(file_spec.GetPath(), std::ios::binary);
+    if (!file.is_open())
+        return false;
+
+    char read_buf[4096];
+    while (!file.eof())
+    {
+        file.read(read_buf, sizeof(read_buf));
+        const auto read_bytes = file.gcount();
+        if (read_bytes == 0)
+            break;
+
+        md5_hash.update(llvm::StringRef(read_buf, read_bytes));
+    }
+
+    llvm::MD5::MD5Result md5_result;
+    md5_hash.final(md5_result);
+    llvm::SmallString<32> result_str;
+    llvm::MD5::stringifyResult(md5_result, result_str);
+    digest_str = result_str.c_str();
+    return true;
+}

source/Host/posix/FileSystem.cpp

FileSystem::Readlink(const char *path, char *buf, size_t buf_len)
     if (count < 0)
         error.SetErrorToErrno();
     else if (static_cast<size_t>(count) < (buf_len - 1))
         buf[count] = '\0'; // Success
     else
         error.SetErrorString("'buf' buffer is too small to contain link contents");
     return error;
 }
-
-bool
-FileSystem::CalculateMD5(const FileSpec &file_spec, uint64_t &low, uint64_t &high)
-{
-#if defined(__APPLE__)
-    StreamString md5_cmd_line;
-    md5_cmd_line.Printf("md5 -q '%s'", file_spec.GetPath().c_str());
-    std::string hash_string;
-    Error err = Host::RunShellCommand(md5_cmd_line.GetData(), NULL, NULL, NULL, &hash_string, 60);
-    if (err.Fail())
-        return false;
-    // a correctly formed MD5 is 16-bytes, that is 32 hex digits
-    // if the output is any other length it is probably wrong
-    if (hash_string.size() != 32)
-        return false;
-    std::string part1(hash_string, 0, 16);
-    std::string part2(hash_string, 16);
-    const char *part1_cstr = part1.c_str();
-    const char *part2_cstr = part2.c_str();
-    high = ::strtoull(part1_cstr, NULL, 16);
-    low = ::strtoull(part2_cstr, NULL, 16);
-    return true;
-#else
-    // your own MD5 implementation here
-    return false;
-#endif
-}

source/Host/windows/FileSystem.cpp

FileSystem::Readlink(const char *path, char *buf, size_t buf_len)
     // Subtract 1 from the path length since this function does not add a null terminator.
     DWORD result = ::GetFinalPathNameByHandle(h, buf, buf_len - 1, FILE_NAME_NORMALIZED | VOLUME_NAME_DOS);
     if (result == 0)
         error.SetError(::GetLastError(), lldb::eErrorTypeWin32);
 
     ::CloseHandle(h);
     return error;
 }
-
-bool
-FileSystem::CalculateMD5(const FileSpec &file_spec, uint64_t &low, uint64_t &high)
-{
-    return false;
-}