[PATCH] D9600: Add scan-build python implementation

Daniel Marjamäki via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 6 23:59:16 PST 2016


danielmarjamaki added a comment.

Some LLVM/Clang coding practices review comments.


================
Comment at: tools/scan-build-py/tests/functional/exec/main.c:115
@@ +114,3 @@
+void call_execv() {
+    char *const file = "execv.c";
+    char *const compiler = "/usr/bin/cc";
----------------
You are assigning a const literal to a non-const pointer which is dangerous. It seems to me you should use 'const char *' instead. personally I like 'const char * const' better but as far as I know that is against normal coding practices in LLVM/Clang.

================
Comment at: tools/scan-build-py/tests/functional/exec/main.c:122
@@ +121,3 @@
+
+    FORK(execv(compiler, argv);)
+}
----------------
I would suggest that you remove the semicolon here and add it in the macro instead. the macro usage will be more "function-like" then.


================
Comment at: tools/scan-build-py/tests/functional/exec/main.c:128
@@ +127,3 @@
+void call_execve() {
+    char *const file = "execve.c";
+    char *const compiler = "/usr/bin/cc";
----------------
according to llvm naming conventions, variable names start with uppercase => File, Compiler, Argv, Envp

================
Comment at: tools/scan-build-py/tests/functional/exec/main.c:133
@@ +132,3 @@
+
+    expected_out(file);
+    create_source(file);
----------------
according to llvm naming conventions, expectedOut and createSource would be better names


http://reviews.llvm.org/D9600





More information about the cfe-commits mailing list