Re: [Libguestfs] Remaining syntax-check errors in libguestfs

Richard W.M. Jones wrote: ... cont'd I would exempt ^examples/ from this check. Thank you. That is another regexp that can be improved: [the ":" was trying to anchor that "#" to beginning of line in the grep-style "FILE:...text" output that it's filtering, but that is wrong for two reasons: your example is one, but the other is that the "#" need not be at the beginning of the line.] diff --git a/top/maint.mk b/top/maint.mk index 4627bc5..f77d0c1 100644 --- a/top/maint.mk +++ b/top/maint.mk @@ -330,7 +330,7 @@ sc_prohibit_atoi_atof: sp_ = strcmp *\(.+\) sc_prohibit_strcmp: @prohibit='! *strcmp *\(|\<$(sp_) *[!=]=|[!=]= *$(sp_)' \ - exclude=':# *define STRN?EQ\(' \ + exclude='# *define STRN?EQ\(' \ halt='replace strcmp calls above with STREQ/STRNEQ' \ $(_sc_search_regexp) ... You got that right ;-) I agree. I would exempt those two files. This is well documented in autoconf's "Limitations of Builtins" documentation: `test' The `test' program is the way to perform many file and string tests. It is often invoked by the alternate name `[', but using that name in Autoconf code is asking for trouble since it is an M4 quote character. The `-a', `-o', `(', and `)' operands are not present in all implementations, and have been marked obsolete by Posix 2008. This is because there are inherent ambiguities in using them. For example, `test "$1" -a "$2"' looks like a binary operator to check whether two strings are both non-empty, but if `$1' is the literal `!', then some implementations of `test' treat it as a negation of the unary operator `-a'. Thus, portable uses of `test' should never have more than four arguments, and scripts should use shell constructs like `&&' and `||' instead. If you combine `&&' and `||' in the same statement, keep in mind that they have equal precedence, so it is often better to parenthesize even when this is redundant. For example: # Not portable: test "X$a" = "X$b" -a \ '(' "X$c" != "X$d" -o "X$e" = "X$f" ')' # Portable: test "X$a" = "X$b" && { test "X$c" != "X$d" || test "X$e" = "X$f"; } `test' does not process options like most other commands do; for example, it does not recognize the `--' argument as marking the end of options. It is safe to use `!' as a `test' operator. For example, `if test ! -d foo; ...' is portable even though `if ! test -d foo; ...' is not. I would exempt those files or change the generation rules to filter out trailing blanks. Of course, it's even better not to VC generated files, but I suppose you have no choice because the tools to perform the generation are not guaranteed to be available. exempt it, I suppose, though it doesn't look like a binary to me. ... Pet peeve ;-) Yes, another case of exemption-required. Yeah, this can be a pain (especially if you do it all at once), but the consolation is that you have to do it only once. Any subsequent failures typically indicate real problems or are quickly dispatched via another exemption. Hey! I've just realized that I can easily filter out those lines. Here's yet another patch. We should be able to use a similar change for a few other rules. diff --git a/top/maint.mk b/top/maint.mk index 4627bc5..ccf09a2 100644 --- a/top/maint.mk +++ b/top/maint.mk @@ -724,6 +724,7 @@ sc_require_test_exit_idiom: sc_trailing_blank: @prohibit='[ ]$$' \ halt='found trailing blank(s)' \ + exclude='^Binary file .* matches$$' \ $(_sc_search_regexp) # Match lines like the following, but where there is only one space Thanks for the detailed report!