[Lldb-commits] [PATCH] D54617: [Reproducers] Add file provider
Davide Italiano via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Fri Jan 25 15:26:55 PST 2019
davide added a comment.
Some comments. Thanks!
Comment at: include/lldb/Utility/FileCollector.h:5-7
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
You might want to update the license.
Comment at: include/lldb/Utility/FileCollector.h:50-54
+ std::mutex m_mutex;
+ FileSpec m_root;
+ llvm::StringSet<> m_seen;
+ llvm::vfs::YAMLVFSWriter m_vfs_writer;
+ llvm::StringMap<std::string> m_symlink_map;
Can we add comments to these member variables? Some are not entirely obvious to me, e.g. what's `m_root`?
What's `m_mutex` is supposed to protect . I think we should document these invariants.
Comment at: source/Utility/FileCollector.cpp:27-33
+ // Change path to all upper case and ask for its real path, if the latter
+ // exists and is equal to path, it's not case sensitive. Default to case
+ // sensitive in the absence of real_path, since this is the YAMLVFSWriter
+ // default.
+ upper_dest = path.upper();
+ if (sys::fs::real_path(upper_dest, real_dest) && path.equals(real_dest))
+ return false;
should this be a function in FS to check the case-sensitiveness?
Comment at: source/Utility/FileCollector.cpp:84-87
+ // Canonicalize the source path by removing "..", "." components.
+ SmallString<256> virtual_path = absolute_src;
+ sys::path::remove_dots(virtual_path, /*remove_dot_dot=*/true);
Ditto (I thought there was one to canonicalize the source path?)
CHANGES SINCE LAST ACTION
More information about the lldb-commits