ovyalov added a comment.
Please see my comments.
================
Comment at: source/Plugins/Platform/Android/PlatformAndroid.cpp:306
@@ +305,3 @@
+ if (!module_sp->GetPlatformFileSpec())
+ return Error("No platform file specified");
+
----------------
indentation.
================
Comment at: source/Plugins/Platform/Android/PlatformAndroid.cpp:325
@@ +324,3 @@
+ &tmpdir,
+ 1);
+
----------------
Please add constant for timeout.
================
Comment at: source/Plugins/Platform/Android/PlatformAndroid.cpp:329
@@ +328,3 @@
+ return Error("Failed to generate temporary directory on the device
(%s)", error.AsCString());
+ tmpdir.erase(tmpdir.size() - 1); // Remove trailing new line
+
----------------
Please check that tmpdir isn't empty.
================
Comment at: source/Plugins/Platform/Android/PlatformAndroid.cpp:331
@@ +330,3 @@
+
+ // Create file remover for the temporary directroy created on the device
+ std::unique_ptr<std::string, std::function<void(std::string*)>>
tmpdir_remover(
----------------
s/directroy/directory
================
Comment at: source/Plugins/Platform/Android/PlatformAndroid.cpp:337
@@ +336,3 @@
+ command.Printf("rm -rf %s", s->c_str());
+ this->RunShellCommand(command.GetData(),
+ GetWorkingDirectory(),
----------------
Please log an error if rm fails.
================
Comment at: source/Plugins/Platform/Android/PlatformAndroid.cpp:342
@@ +341,3 @@
+ nullptr,
+ 1);
+ }
----------------
Can we have a bigger timeout? 1 sec might not be enough.
================
Comment at: source/Plugins/Platform/Android/PlatformAndroid.cpp:352
@@ +351,3 @@
+ command.Printf("oatdump --symbolize=%s --output=%s",
+ module_sp->GetPlatformFileSpec().GetCString(),
+ symfile_platform_filespec.GetCString());
----------------
GetCString(false)?
================
Comment at: source/Plugins/Platform/Android/PlatformAndroid.cpp:353
@@ +352,3 @@
+ module_sp->GetPlatformFileSpec().GetCString(),
+ symfile_platform_filespec.GetCString());
+ error = RunShellCommand(command.GetData(),
----------------
ditto - please verify that it works as expected from Windows.
================
Comment at: source/Utility/ModuleCache.cpp:137
@@ +136,3 @@
+
+ FileSpec symfile_spec((cached_module_sp->GetFileSpec ().GetPath () +
".sym").c_str (), false);
+ if (symfile_spec.Exists ())
----------------
Please add ".sym" constant
================
Comment at: source/Utility/ModuleCache.cpp:197
@@ +196,3 @@
+
+ // Fetching a symbol file for the modul
+ const auto tmp_download_sym_file_spec = JoinPath (module_spec_dir,
kTempSymFileName);
----------------
s/modul/module
================
Comment at: source/Utility/ModuleCache.cpp:207
@@ +206,3 @@
+ FileSpec symfile_spec((cached_module_sp->GetFileSpec ().GetPath () +
".sym").c_str (), false);
+ error = Put (root_dir_spec, hostname, module_spec,
tmp_download_sym_file_spec, symfile_spec);
+ if (error.Fail ())
----------------
Do we really want to have a link for sym file in sys root?
I'd rather moved code for sym file download from GetAndPut into Put method and
skip link creation for symbol file.
http://reviews.llvm.org/D11936
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
