[Lldb-commits] [PATCH] D11936: Download symbol file for .oat files on android
Tamas Berghammer via lldb-commits
lldb-commits at lists.llvm.org
Tue Aug 11 10:00:06 PDT 2015
tberghammer added a comment.
See responds inline
Comment at: source/Plugins/Platform/Android/PlatformAndroid.cpp:325
@@ +324,3 @@
> Please add constant for timeout.
Added a comment next to the value instead. I prefer that style but I can use a constant if you want.
Comment at: source/Plugins/Platform/Android/PlatformAndroid.cpp:342
@@ +341,3 @@
> Can we have a bigger timeout? 1 sec might not be enough.
I think 1 second should be enough in almost every case but increase it to 5 seconds just in case the device is very busy.
Comment at: source/Plugins/Platform/Android/PlatformAndroid.cpp:353
@@ +352,3 @@
+ error = RunShellCommand(command.GetData(),
> ditto - please verify that it works as expected from Windows.
At the moment I don't have Windows environment but I will try to find a way to test it.
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.
At the moment I think we don't use the sys root at all, but I might be wrong. If anywhere we use the sys root then we will need the symbol file next to the actual file to find it. The reason I put the file download here because we need a Module object to do it (find out if we have symtab) what means we have to do it after the Get. I think moving it to Put will make the implementation more complicated.
More information about the lldb-commits