[PATCH 0/6] whitespace & '-D' option

[PATCH 0/6] whitespace & '-D' option

[Luc Van Oostenryck]

This series contains two fixes concerning the '-D' option
and white spaces. A a bonus it also contains an improvement
to the testsuite which now respect quotes/whitespace in
commands' options.

Luc Van Oostenryck (6):
  add testcase for 'sparse -D M...'
  fix: accept 'sparse -D M...'
  testsuite: add test case for quoting of command's arguments
  testsuite: respect command line's quotes & whitespaces
  add test case for space within command line
  fix: spaces in macro definition on the command line

-- 
2.15.0

[PATCH 1/6] add testcase for 'sparse -D M...'

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/preprocessor/cli-D-arg.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100644 validation/preprocessor/cli-D-arg.c

diff --git a/validation/preprocessor/cli-D-arg.c b/validation/preprocessor/cli-D-arg.c
new file mode 100644
index 000000000..b098e98bd
--- /dev/null
+++ b/validation/preprocessor/cli-D-arg.c
@@ -0,0 +1,13 @@
+A
+B
+/*
+ * check-name: cli: -D MACRO
+ * check-command: sparse -E -D A -D B=abc $file
+ * check-known-to-fail
+ *
+ * check-output-start
+
+1
+abc
+ * check-output-end
+ */
-- 
2.15.0

[PATCH 2/6] fix: accept 'sparse -D M...'

[Luc Van Oostenryck]

Till now, sparse was unneedlessly strict in what it accepted in
'-D' options. More specifically, it doesn't accept:
1) separated '-D' and the macro definition, like:
	sparse -D MACRO[=definition] ...
2) a space between the '-D' and the macro name, like:
	sparse '-D MACRO[=definition] ...

Case 1) is clearly accepted by GCC, clang and should be
accepted for a POSIX's c99. Case 2's status is less clear
but is also accepted by GCC and clang (leaving any validation
to the corresponding internal #define).

Fix this by accepting separated command line argument for '-D'
and the macro (and removing the check that rejected the macro
part if it started with a space).

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 lib.c                               | 12 ++++++++----
 validation/preprocessor/cli-D-arg.c |  1 -
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/lib.c b/lib.c
index 037206f72..236d58fc9 100644
--- a/lib.c
+++ b/lib.c
@@ -323,12 +323,16 @@ static char **handle_switch_D(char *arg, char **next)
 	const char *name = arg + 1;
 	const char *value = "1";
 
-	if (!*name || isspace((unsigned char)*name))
-		die("argument to `-D' is missing");
+	if (!*name) {
+		arg = *++next;
+		if (!arg)
+			die("argument to `-D' is missing");
+		name = arg;
+	}
 
-	for (;;) {
+	for (;;arg++) {
 		char c;
-		c = *++arg;
+		c = *arg;
 		if (!c)
 			break;
 		if (isspace((unsigned char)c) || c == '=') {
diff --git a/validation/preprocessor/cli-D-arg.c b/validation/preprocessor/cli-D-arg.c
index b098e98bd..03c5bac34 100644
--- a/validation/preprocessor/cli-D-arg.c
+++ b/validation/preprocessor/cli-D-arg.c
@@ -3,7 +3,6 @@ B
 /*
  * check-name: cli: -D MACRO
  * check-command: sparse -E -D A -D B=abc $file
- * check-known-to-fail
  *
  * check-output-start
 
-- 
2.15.0

[PATCH 3/6] testsuite: add test case for quoting of command's arguments

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/self-quote-args.c | 8 ++++++++
 1 file changed, 8 insertions(+)
 create mode 100644 validation/self-quote-args.c

diff --git a/validation/self-quote-args.c b/validation/self-quote-args.c
new file mode 100644
index 000000000..acac41d7d
--- /dev/null
+++ b/validation/self-quote-args.c
@@ -0,0 +1,8 @@
+/*
+ * check-name: self-quote-args
+ * check-description: This is testing that the test-suite
+ *	respect the quoting of the command's arguments.
+ * check-command: sparse '-foption with-spaces' empty-file
+ * check-known-to-fail
+ * check-output-ignore
+ */
-- 
2.15.0

[PATCH 4/6] testsuite: respect command line's quotes & whitespaces

[Luc Van Oostenryck]

Currently the testsuite use 'eval echo $cmd' to expand
the name of the test file to be given on the command line.

This has the annoying consequence to go a bit too far in
the expansion of variables and to destroy any quotes and
whitespaces escaping that would have done.

Fix this by doing the eval later, when effectively executing
the command.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/self-quote-args.c |  1 -
 validation/test-suite        | 10 +++++-----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/validation/self-quote-args.c b/validation/self-quote-args.c
index acac41d7d..be9873d23 100644
--- a/validation/self-quote-args.c
+++ b/validation/self-quote-args.c
@@ -3,6 +3,5 @@
  * check-description: This is testing that the test-suite
  *	respect the quoting of the command's arguments.
  * check-command: sparse '-foption with-spaces' empty-file
- * check-known-to-fail
  * check-output-ignore
  */
diff --git a/validation/test-suite b/validation/test-suite
index 192fba300..dba8fd50c 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -308,25 +308,25 @@ do_test()
 		fi
 	fi
 
-	cmd=`eval echo $default_path/$check_command`
-
 	if [ -z "$vquiet" ]; then
 		echo "  TEST    $test_name ($file)"
 	fi
 
-	verbose "Using command       : $cmd"
+	verbose "Using command       : $(echo "$@")"
 
 	# grab the expected exit value
 	expected_exit_value=$check_exit_value
 	verbose "Expecting exit value: $expected_exit_value"
 
 	# do we want a timeout?
+	pre=""
 	if [ $check_timeout -ne 0 ]; then
-		cmd="timeout -k 1s $check_timeout $cmd"
+		pre="timeout -k 1s $check_timeout"
 	fi
 
 	# grab the actual output & exit value
-	$cmd 1> $file.output.got 2> $file.error.got
+	shift
+	eval $pre ../$base_cmd "$@" 1> $file.output.got 2> $file.error.got
 	actual_exit_value=$?
 
 	must_fail=$check_known_to_fail
-- 
2.15.0

[PATCH 5/6] add test case for space within command line

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/preprocessor/cli-D-space.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100644 validation/preprocessor/cli-D-space.c

diff --git a/validation/preprocessor/cli-D-space.c b/validation/preprocessor/cli-D-space.c
new file mode 100644
index 000000000..d104d66da
--- /dev/null
+++ b/validation/preprocessor/cli-D-space.c
@@ -0,0 +1,11 @@
+M(0,1)
+/*
+ * check-name: cli: allow spaces in macros
+ * check-command: sparse -E '-DM(X, Y)=a' $file
+ * check-known-to-fail
+ *
+ * check-output-start
+
+a
+ * check-output-end
+ */
-- 
2.15.0

[PATCH 6/6] fix: spaces in macro definition on the command line

[Luc Van Oostenryck]

GCC's manual or POSIX say about the '-D' option something like:
    '−D name[=value]' should be treated as if in directive
    '#define name value' (with '1' as default for the value),
    including its tokenization.
So an option like '-DM(X, Y)=...' should be processed like a
directive '#define M(X, Y) ...'.

However, the current code treat a space as a separator between
the macro and its definition, just like the '='. As consequence,
the above option is processed like the directive would be
'#define M(X, Y)=...', with 'M(X,' as the macro (name) and
'Y)=...' as its definition.

Fix this by stopping to treat the space character specially,
thus only using '=' as the separator.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 lib.c                                 | 2 +-
 validation/preprocessor/cli-D-space.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib.c b/lib.c
index 236d58fc9..e4bb639e5 100644
--- a/lib.c
+++ b/lib.c
@@ -335,7 +335,7 @@ static char **handle_switch_D(char *arg, char **next)
 		c = *arg;
 		if (!c)
 			break;
-		if (isspace((unsigned char)c) || c == '=') {
+		if (c == '=') {
 			*arg = '\0';
 			value = arg + 1;
 			break;
diff --git a/validation/preprocessor/cli-D-space.c b/validation/preprocessor/cli-D-space.c
index d104d66da..8343bf1ac 100644
--- a/validation/preprocessor/cli-D-space.c
+++ b/validation/preprocessor/cli-D-space.c
@@ -2,7 +2,6 @@ M(0,1)
 /*
  * check-name: cli: allow spaces in macros
  * check-command: sparse -E '-DM(X, Y)=a' $file
- * check-known-to-fail
  *
  * check-output-start
 
-- 
2.15.0

Re: [PATCH 0/6] whitespace & '-D' option

[Christopher Li]

On Thu, Dec 14, 2017 at 1:15 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> This series contains two fixes concerning the '-D' option
> and white spaces. A a bonus it also contains an improvement
> to the testsuite which now respect quotes/whitespace in
> commands' options.
>
> Luc Van Oostenryck (6):
>   add testcase for 'sparse -D M...'
>   fix: accept 'sparse -D M...'
>   testsuite: add test case for quoting of command's arguments
>   testsuite: respect command line's quotes & whitespaces
>   add test case for space within command line
>   fix: spaces in macro definition on the command line

Hi Luc,

I will be traveling for the whole next week.

No sure I will have time to get on it in the
follow week. I will take a look when I get back.

Chris

Re: [PATCH 0/6] whitespace & '-D' option

[Luc Van Oostenryck]

On Fri, Dec 15, 2017 at 02:15:17PM +0800, Christopher Li wrote:
> On Thu, Dec 14, 2017 at 1:15 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > This series contains two fixes concerning the '-D' option
> > and white spaces. A a bonus it also contains an improvement
> > to the testsuite which now respect quotes/whitespace in
> > commands' options.
> >
> > Luc Van Oostenryck (6):
> >   add testcase for 'sparse -D M...'
> >   fix: accept 'sparse -D M...'
> >   testsuite: add test case for quoting of command's arguments
> >   testsuite: respect command line's quotes & whitespaces
> >   add test case for space within command line
> >   fix: spaces in macro definition on the command line
> 
> Hi Luc,
> 
> I will be traveling for the whole next week.
> 
> No sure I will have time to get on it in the
> follow week. I will take a look when I get back.

No worries.
-- Luc