-
Notifications
You must be signed in to change notification settings - Fork 1.9k
C++: new query for futile arguments to C functions #790
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
64ed930
fa02042
44d8e6b
54fdf9f
9642a78
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| void no_argument(); | ||
|
|
||
| void one_argument(int x); | ||
|
|
||
| void calls() { | ||
| no_argument(1) // BAD: `no_argument` will accept and ignore the argument | ||
|
|
||
| one_argument(1); // GOOD: `one_argument` will accept and use the argument | ||
|
|
||
| no_argument(); // GOOD: `no_argument` has not been passed an argument | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| <!DOCTYPE qhelp PUBLIC | ||
| "-//Semmle//qhelp//EN" | ||
| "qhelp.dtd"> | ||
| <qhelp> | ||
|
|
||
|
|
||
| <overview> | ||
| <p>A function is called with arguments despite having an empty parameter list. This may indicate | ||
| that the incorrect function is being called, or that the author misunderstood the function.</p> | ||
|
|
||
| <p>In C, a function declared with an empty parameter list `()` is considered to have an unknown | ||
| parameter list, and therefore can be called with any set of arguments. To declare a function | ||
| which takes no arguments, you must use `(void)` as the parameter list in any forward declarations. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what you mean by "forward" here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes; a forward declaration is a declaration that is not a definition (for functions, this means the declaration does not contain a function body) |
||
| In C++, either style of declaration indicates that the function accepts no arguments.</p> | ||
|
|
||
| </overview> | ||
| <recommendation> | ||
| <p>Call the function without arguments, or call a different function that expects the arguments | ||
| being passed.</p> | ||
|
|
||
| </recommendation> | ||
| <example><sample src="FutileParams.c" /> | ||
|
|
||
| </example> | ||
|
|
||
| <references> | ||
| <li>SEI CERT C++ Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/DCL20-C.+Explicitly+specify+void+when+a+function+accepts+no+arguments"> DCL20-C. Explicitly specify void when a function accepts no arguments </a></li> | ||
| </references> | ||
| </qhelp> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| /** | ||
| * @name Non-empty call to function declared without parameters | ||
| * @description A call to a function declared without parameters has arguments, which may indicate | ||
| * that the code does not follow the author's intent. | ||
| * @kind problem | ||
| * @problem.severity warning | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From the results you've shown so far, I think we can make it at least
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In addition to @precision, add @id and @tags attributes? |
||
| * @precision very-high | ||
| * @id cpp/futile-params | ||
| * @tags correctness | ||
| * maintainability | ||
| */ | ||
|
|
||
| import cpp | ||
|
|
||
| from Function f, FunctionCall fc | ||
| where fc.getTarget() = f | ||
| and f.getNumberOfParameters() = 0 | ||
| and not f.isVarargs() | ||
| and fc.getNumberOfArguments() != 0 | ||
| and not f instanceof BuiltInFunction | ||
| and exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() | not fde.isImplicit()) | ||
| select fc, "This call has arguments, but $@ is not declared with any parameters.", f, f.toString() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| | test.c:8:3:8:16 | call to declared_empty | This call has arguments, but $@ is not declared with any parameters. | test.c:1:6:1:19 | declared_empty | declared_empty | | ||
| | test.c:14:3:14:19 | call to not_yet_declared1 | This call has arguments, but $@ is not declared with any parameters. | test.c:14:3:14:3 | not_yet_declared1 | not_yet_declared1 | | ||
| | test.c:14:3:14:19 | call to not_yet_declared1 | This call has arguments, but $@ is not declared with any parameters. | test.c:25:6:25:22 | not_yet_declared1 | not_yet_declared1 | |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Likely Bugs/Likely Typos/FutileParams.ql |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| void declared_empty(); | ||
| void declared_void(void); | ||
| void declared_with(int); | ||
| void declared_empty_defined_with(); | ||
|
|
||
| void test() { | ||
| declared_empty(); // GOOD | ||
| declared_empty(1); // BAD | ||
| declared_void(); // GOOD | ||
| declared_with(1); // GOOD | ||
|
|
||
| undeclared(1); // GOOD | ||
|
|
||
| not_yet_declared1(1); // BAD | ||
| not_yet_declared2(1); // GOOD | ||
|
|
||
| declared_empty_defined_with(); // BAD [NOT DETECTED] | ||
| declared_empty_defined_with(1); // GOOD | ||
|
|
||
| int x; | ||
| declared_empty_defined_with(&x); // BAD [NOT DETECTED] | ||
| declared_empty_defined_with(x, x); // BAD [NOT DETECTED] | ||
| } | ||
|
|
||
| void not_yet_declared1(); | ||
| void not_yet_declared2(int); | ||
| void declared_empty_defined_with(int x) { | ||
| // do nothing | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| void cpp_varargs(...); | ||
| void bar(); | ||
|
|
||
| void test() { | ||
| cpp_varargs(); // GOOD | ||
| cpp_varargs(1); // GOOD | ||
| __builtin_constant_p("something"); // GOOD: builtin | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add text to explain the difference between
()in C,(void)in C/C++, and()in C++.