[Lldb-commits] [PATCH] Introduce FileSystem::CalculateMD5AsString that supports any platform and make existing FileSystem::CalculateMD5 to use it.

Zachary Turner zturner at google.com
Thu Feb 19 15:06:34 PST 2015


================
Comment at: source/Host/common/FileSystem.cpp:22
@@ +21,3 @@
+{
+    std::string hash_string;
+    if (!CalculateMD5AsString(file_spec, hash_string))
----------------
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.

================
Comment at: source/Host/common/FileSystem.cpp:46
@@ +45,3 @@
+
+    char read_buf[4096];
+    while (!file.eof())
----------------
I would prefer if we used a vector<char> here.  Taking up 4K of stack is a little bit obnoxious for a simple function.

================
Comment at: source/Host/common/FileSystem.cpp:61
@@ +60,3 @@
+    llvm::MD5::stringifyResult(md5_result, result_str);
+    digest_str = result_str.c_str();
+    return true;
----------------
Similarly, with the aforementioned change, this function becomes:

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

http://reviews.llvm.org/D7771

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the lldb-commits mailing list