> I have been working with Jan-Oliver Wagner on implementing CR#57[1] to
> link vulnerability detection results to the corresponding product
> detection results.
> Please find the corresponding patches attached, for comments:
Just some small coding style comments.
- Please keep line width to max of 80 in the C code.
- The function and macro need docs. Macro will have to go outside
function for this.
- Saw some "sql_string(0". Should be a space after "g".
- In binary operations like comparisons over multiple lines or where the line
is very long, please put the operator at the beginning of the following
line. Otherwise it's hard to read the operation. So
+ if ((report = sql_string (0, 0,
+ "SELECT report FROM report_results
where result = %llu;",
+ result)) == NULL)
like
+ if ((report = sql_string (0, 0,
+ "SELECT report FROM report_results
where result = %llu;",
+ result))
+ == NULL)
And in this example the assignment can go outside the conditional which
will save the extra parens and the long line, and will be clearer.
- In the SQL please put the paren on the same line as the expression it
starts. And it's nice if the WHERE is at the same position as any ANDs.
&& sql_int (0, 0,
"SELECT count(*)"
" FROM overrides"
" WHERE ((overrides.owner IS NULL)"
" OR (overrides.owner ="
" (SELECT ROWID FROM users"
" WHERE users.uuid = '%s')))"
" AND ((overrides.end_time = 0)"
" OR (overrides.end_time >= now ()))",
current_credentials.uuid))
- Maybe duplicate the cleanup, instead of doing the complex return?
+ goto detect_cleanup;
+
+ g_free (report);
+ g_free (host);
+
+ return 0;
+
+detect_cleanup:
+ g_free (report);
+ g_free (host);
+
+ return -1;
+}
- In Manager I've consistently used explicit NULL comparison instead of
negation: ref == NULL || product == NULL ...
- Personally I like clearer variable names (detail_ref vs d_ref),
especially when the name is still quite short.
--
Greenbone Networks GmbH
Neuer Graben 17, 49074 Osnabrueck, Germany | AG Osnabrueck, HR B 202460
Executive Directors: Lukas Grunwald, Dr. Jan-Oliver Wagner
_______________________________________________
Openvas-devel mailing list
Openvas-devel@wald.intevation.org
http://lists.wald.intevation.org/mailman/listinfo/openvas-devel
