bryant added inline comments.
================ Comment at: CMakeLists.txt:46 + COMMAND + ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py + --srcRoot=${LLDB_SOURCE_DIR} ---------------- You can reduce diff noise by leaving formatting changes for a separate patch. ================ Comment at: scripts/CMakeLists.txt:38 + COMMAND + ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/prepare_bindings.py + ${framework_arg} ---------------- Move this back up. ================ Comment at: scripts/Python/prepare_binding_Python.py:222 + ) + logging.info("running swig with: %r", command) ---------------- You can reduce diff noise by limiting your changes to removing the %s. So, ```python # Build the SWIG args list options.swig_executable, "-c++", "-shadow", "-python", "-threads", "-I" + os.path.normcase( os.path.join(options.src_root, "include")), "-I" + os.path.normcase("./."), "-D__STDC_LIMIT_MACROS", "-D__STDC_CONSTANT_MACROS"] if options.target_platform == "Darwin": command.append("-D__APPLE__") if options.generate_dependency_file: command.extend(["-MMD", " -MF", temp_dep_file_path]) command.extend([ "-outdir", config_build_dir, "-o", settings.output_file, settings.input_file ]) logging.info("running swig with: %s", command) ``` ================ Comment at: scripts/Python/prepare_binding_Python.py:229 stderr=subprocess.PIPE, - shell=True) + ) # Wait for SWIG process to terminate ---------------- ldrumm wrote: > granata.enrico wrote: > > This worries me a little bit.. Are we sure we are not in any way relying on > > the shell in executing the command line? > The features of the shell are not used in this expression at all, and the > environment is identical to the previous invocation. `shell=False` for both python 2 and 3: https://docs.python.org/2/library/subprocess.html#subprocess.Popen ; https://docs.python.org/3/library/subprocess.html#subprocess.Popen , unless I've missed your meaning. https://reviews.llvm.org/D26757 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits