[PATCH] D31695: Windows asan_device_setup.bat port of linux shell script
Evgenii Stepanov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 28 14:33:03 PDT 2017
eugenis added inline comments.
================
Comment at: asan_device_setup.py:53
+
+ def adbCmd(self, cmd):
+ cmd = "adb " + cmd
----------------
Consider moving the implementation of basic actions (push, pull, shell, maybe install/chmod/chown) into a separate class (maybe Device), or at least grouping them near the beginning of the file.
================
Comment at: asan_device_setup.py:113
+ self.ASAN_RT_PATH=""
+ if os.path.isfile(os.path.join(HERE, "..", "lib", self.ASAN_RT)):
+ self.ASAN_RT_PATH = os.path.normpath(os.path.join(HERE, "..", "lib"))
----------------
This is hard to read. Also, each path is mentioned twice. Please build an array of paths and loop over it.
================
Comment at: asan_device_setup.py:118
+ if os.path.isfile(os.path.join(HERE, "..", "lib", "linux", self.ASAN_RT)):
+ self.ASAN_RT_PATH = os.path.normpath(os.path.join(HERE, "..", "lib"))
+ for subdir, dirs, files in os.walk(os.path.join(HERE, "..", "lib", "clang")):
----------------
I think this line is missing "linux".
================
Comment at: asan_device_setup.py:184
+ shutil.move(TMPDIR + "/app_process64", TMPDIR + "/app_process64.real")
+ self.generateZygotWrapper(TMPDIR + "/app_process32", "/system/bin/app_process32.real", self.ASAN_RT)
+ self.generateZygotWrapper(TMPDIR + "/app_process64", "/system/bin/app_process64.real", self.ASAN_RT64)
----------------
s/Zygot/Zygote/
================
Comment at: asan_device_setup.py:191
+ # zygote).
+ outFile = open(TMPDIR + "/asanwrapper", "w")
+ outFile.truncate()
----------------
Move this into a helper function.
================
Comment at: asan_device_setup.py:259
+
+ def parseArgsLoop(self):
+ argList = sys.argv[1:]
----------------
Is there a reason you are not using standard python argument parsing libraries?
https://reviews.llvm.org/D31695
More information about the llvm-commits
mailing list