[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