linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* sparse: new feature " multiple initializers" has false positives on MODULE_ALIAS
@ 2015-01-22 20:31 Christian Borntraeger
  2015-01-23 16:40 ` Christopher Li
  0 siblings, 1 reply; 20+ messages in thread
From: Christian Borntraeger @ 2015-01-22 20:31 UTC (permalink / raw)
  To: Linus Torvalds, Christopher Li; +Cc: Jason J. Herne, linux-sparse

Linus, Christopher,

Commit  0f25c6a78e08fdc15af5e599d836fa24349c042f ("Add warning about duplicate initializers") has a false positive on arch/s390/kvm/kvm-s390.c

  CHECK   arch/s390/kvm/kvm-s390.c
arch/s390/kvm/kvm-s390.c:1823:1: error: symbol '__UNIQUE_ID_alias__COUNTER__' has multiple initializers (originally initialized at arch/s390/kvm/kvm-s390.c:1822)

1822: MODULE_ALIAS_MISCDEV(KVM_MINOR);
1823: MODULE_ALIAS("devname:kvm");


Preprocessing with gcc gives
static const char __UNIQUE_ID_alias0[] __attribute__((__used__)) __attribute__((section(".modinfo"), unused, aligned(1))) = "alias" "=" "char-major-" "10" "-" "232";
static const char __UNIQUE_ID_alias1[] __attribute__((__used__)) __attribute__((section(".modinfo"), unused, aligned(1))) = "alias" "=" "devname:kvm";

so alias0 and alias1 instead of __COUNTER__.
I never heard of __COUNTER__ before, so I guess its some gcc magic that sparse should mimic..

Christian


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: sparse: new feature " multiple initializers" has false positives on MODULE_ALIAS
  2015-01-22 20:31 sparse: new feature " multiple initializers" has false positives on MODULE_ALIAS Christian Borntraeger
@ 2015-01-23 16:40 ` Christopher Li
  2015-01-23 22:23   ` [PATCH] Teach sparse about the __COUNTER__ predefined macro Luc Van Oostenryck
  0 siblings, 1 reply; 20+ messages in thread
From: Christopher Li @ 2015-01-23 16:40 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: Linus Torvalds, Jason J. Herne, Linux-Sparse

On Thu, Jan 22, 2015 at 12:31 PM, Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
> Linus, Christopher,
>
> Commit  0f25c6a78e08fdc15af5e599d836fa24349c042f ("Add warning about duplicate initializers") has a false positive on arch/s390/kvm/kvm-s390.c
>
>   CHECK   arch/s390/kvm/kvm-s390.c
> arch/s390/kvm/kvm-s390.c:1823:1: error: symbol '__UNIQUE_ID_alias__COUNTER__' has multiple initializers (originally initialized at arch/s390/kvm/kvm-s390.c:1822)

Search the "__COUNTER__" macro shows that:
https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html

__COUNTER__This macro expands to sequential integral values starting
from 0. In conjunction with the ## operator, this provides a
convenient means to generate unique identifiers. Care must be taken to
ensure that __COUNTER__ is not expanded prior to inclusion of
precompiled headers which use it. Otherwise, the precompiled headers
will not be used.

I think sparse haven't implement the __COUNTER__ macro. That is why it emit the
error on duplicate entry.

Chris

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] Teach sparse about the __COUNTER__ predefined macro
  2015-01-23 16:40 ` Christopher Li
@ 2015-01-23 22:23   ` Luc Van Oostenryck
  2015-01-23 22:28     ` Sam Ravnborg
  2015-01-23 22:38     ` josh
  0 siblings, 2 replies; 20+ messages in thread
From: Luc Van Oostenryck @ 2015-01-23 22:23 UTC (permalink / raw)
  To: Christopher Li
  Cc: Christian Borntraeger, Linus Torvalds, Jason J. Herne,
	Linux-Sparse

On Fri, Jan 23, 2015 at 08:40:17AM -0800, Christopher Li wrote:
> On Thu, Jan 22, 2015 at 12:31 PM, Christian Borntraeger
> <borntraeger@de.ibm.com> wrote:
> > Linus, Christopher,
> >
> > Commit  0f25c6a78e08fdc15af5e599d836fa24349c042f ("Add warning about duplicate initializers") has a false positive on arch/s390/kvm/kvm-s390.c
> >
> >   CHECK   arch/s390/kvm/kvm-s390.c
> > arch/s390/kvm/kvm-s390.c:1823:1: error: symbol '__UNIQUE_ID_alias__COUNTER__' has multiple initializers (originally initialized at arch/s390/kvm/kvm-s390.c:1822)
> 
> Search the "__COUNTER__" macro shows that:
> https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html
> 
> __COUNTER__This macro expands to sequential integral values starting
> from 0. In conjunction with the ## operator, this provides a
> convenient means to generate unique identifiers. Care must be taken to
> ensure that __COUNTER__ is not expanded prior to inclusion of
> precompiled headers which use it. Otherwise, the precompiled headers
> will not be used.
> 
> I think sparse haven't implement the __COUNTER__ macro. That is why it emit the
> error on duplicate entry.
> 
> Chris
> --


The following patch should fix that.


Luc


Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 ident-list.h                          |  1 +
 pre-process.c                         |  4 ++++
 validation/preprocessor/__COUNTER__.c | 12 ++++++++++++
 3 files changed, 17 insertions(+)
 create mode 100644 validation/preprocessor/__COUNTER__.c

diff --git a/ident-list.h b/ident-list.h
index d5a145f8..b65b667d 100644
--- a/ident-list.h
+++ b/ident-list.h
@@ -108,6 +108,7 @@ __IDENT(__TIME___ident, "__TIME__", 0);
 __IDENT(__func___ident, "__func__", 0);
 __IDENT(__FUNCTION___ident, "__FUNCTION__", 0);
 __IDENT(__PRETTY_FUNCTION___ident, "__PRETTY_FUNCTION__", 0);
+__IDENT(__COUNTER___ident, "__COUNTER__", 0);
 
 /* Sparse commands */
 IDENT_RESERVED(__context__);
diff --git a/pre-process.c b/pre-process.c
index 1aa3d2c4..316247ac 100644
--- a/pre-process.c
+++ b/pre-process.c
@@ -181,6 +181,10 @@ static int expand_one_symbol(struct token **list)
 			time(&t);
 		strftime(buffer, 9, "%T", localtime(&t));
 		replace_with_string(token, buffer);
+	} else if (token->ident == &__COUNTER___ident) {
+		static int counter;
+
+		replace_with_integer(token, counter++);
 	}
 	return 1;
 }
diff --git a/validation/preprocessor/__COUNTER__.c b/validation/preprocessor/__COUNTER__.c
new file mode 100644
index 00000000..98187ee6
--- /dev/null
+++ b/validation/preprocessor/__COUNTER__.c
@@ -0,0 +1,12 @@
+__COUNTER__
+__COUNTER__
+/*
+ * check-name: __COUNTER__ #1
+ * check-command: sparse -E $file
+ *
+ * check-output-start
+
+0
+1
+ * check-output-end
+ */

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro
  2015-01-23 22:23   ` [PATCH] Teach sparse about the __COUNTER__ predefined macro Luc Van Oostenryck
@ 2015-01-23 22:28     ` Sam Ravnborg
  2015-01-23 22:38     ` josh
  1 sibling, 0 replies; 20+ messages in thread
From: Sam Ravnborg @ 2015-01-23 22:28 UTC (permalink / raw)
  To: Luc Van Oostenryck
  Cc: Christopher Li, Christian Borntraeger, Linus Torvalds,
	Jason J. Herne, Linux-Sparse

On Fri, Jan 23, 2015 at 11:23:32PM +0100, Luc Van Oostenryck wrote:
> On Fri, Jan 23, 2015 at 08:40:17AM -0800, Christopher Li wrote:
> > On Thu, Jan 22, 2015 at 12:31 PM, Christian Borntraeger
> > <borntraeger@de.ibm.com> wrote:
> > > Linus, Christopher,
> > >
> > > Commit  0f25c6a78e08fdc15af5e599d836fa24349c042f ("Add warning about duplicate initializers") has a false positive on arch/s390/kvm/kvm-s390.c
> > >
> > >   CHECK   arch/s390/kvm/kvm-s390.c
> > > arch/s390/kvm/kvm-s390.c:1823:1: error: symbol '__UNIQUE_ID_alias__COUNTER__' has multiple initializers (originally initialized at arch/s390/kvm/kvm-s390.c:1822)
> > 
> > Search the "__COUNTER__" macro shows that:
> > https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html
> > 
> > __COUNTER__This macro expands to sequential integral values starting
> > from 0. In conjunction with the ## operator, this provides a
> > convenient means to generate unique identifiers. Care must be taken to
> > ensure that __COUNTER__ is not expanded prior to inclusion of
> > precompiled headers which use it. Otherwise, the precompiled headers
> > will not be used.
> > 
> > I think sparse haven't implement the __COUNTER__ macro. That is why it emit the
> > error on duplicate entry.
> > 
> > Chris
> > --
> 
> 
> The following patch should fix that.

Seems we were working on this in parallel :-)
> 
> 
> Luc
> 
> 
> Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro
  2015-01-23 22:23   ` [PATCH] Teach sparse about the __COUNTER__ predefined macro Luc Van Oostenryck
  2015-01-23 22:28     ` Sam Ravnborg
@ 2015-01-23 22:38     ` josh
  2015-01-23 23:59       ` Luc Van Oostenryck
  1 sibling, 1 reply; 20+ messages in thread
From: josh @ 2015-01-23 22:38 UTC (permalink / raw)
  To: Luc Van Oostenryck
  Cc: Christopher Li, Christian Borntraeger, Linus Torvalds,
	Jason J. Herne, Linux-Sparse

On Fri, Jan 23, 2015 at 11:23:32PM +0100, Luc Van Oostenryck wrote:
> On Fri, Jan 23, 2015 at 08:40:17AM -0800, Christopher Li wrote:
> > I think sparse haven't implement the __COUNTER__ macro. That is why it emit the
> > error on duplicate entry.
> 
> The following patch should fix that.
[...]
> Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

One issue below.

> --- a/pre-process.c
> +++ b/pre-process.c
> @@ -181,6 +181,10 @@ static int expand_one_symbol(struct token **list)
>  			time(&t);
>  		strftime(buffer, 9, "%T", localtime(&t));
>  		replace_with_string(token, buffer);
> +	} else if (token->ident == &__COUNTER___ident) {
> +		static int counter;
> +
> +		replace_with_integer(token, counter++);

This should not use a static counter.  GCC and Sparse can run over more
than one file in one invocation:

$ head test1.c test2.c
==> test1.c <==
__COUNTER__
__COUNTER__

==> test2.c <==
__COUNTER__
__COUNTER__

$ gcc -E test1.c test2.c
# 1 "test1.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "<command-line>" 2
# 1 "test1.c"
0
1
# 1 "test2.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "<command-line>" 2
# 1 "test2.c"
0
1

Notice that the second file starts with __COUNTER__ at 0 again.

The counter *should* keep counting through include files, but needs to
reset before starting a new top-level compile.

- Josh Triplett

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro
  2015-01-23 22:38     ` josh
@ 2015-01-23 23:59       ` Luc Van Oostenryck
  2015-01-24  1:29         ` Josh Triplett
                           ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Luc Van Oostenryck @ 2015-01-23 23:59 UTC (permalink / raw)
  To: josh
  Cc: Christopher Li, Christian Borntraeger, Linus Torvalds,
	Jason J. Herne, Linux-Sparse

On Fri, Jan 23, 2015 at 02:38:56PM -0800, josh@joshtriplett.org wrote:
> On Fri, Jan 23, 2015 at 11:23:32PM +0100, Luc Van Oostenryck wrote:
> > On Fri, Jan 23, 2015 at 08:40:17AM -0800, Christopher Li wrote:
> > > I think sparse haven't implement the __COUNTER__ macro. That is why it emit the
> > > error on duplicate entry.
> > 
> > The following patch should fix that.
> [...]
> > Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
> > 
> > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> 
> One issue below.
> 
> > --- a/pre-process.c
> > +++ b/pre-process.c
> > @@ -181,6 +181,10 @@ static int expand_one_symbol(struct token **list)
> >  			time(&t);
> >  		strftime(buffer, 9, "%T", localtime(&t));
> >  		replace_with_string(token, buffer);
> > +	} else if (token->ident == &__COUNTER___ident) {
> > +		static int counter;
> > +
> > +		replace_with_integer(token, counter++);
> 
> This should not use a static counter.  GCC and Sparse can run over more
> than one file in one invocation:
> 
> $ head test1.c test2.c
> ==> test1.c <==
> __COUNTER__
> __COUNTER__
> 
> ==> test2.c <==
> __COUNTER__
> __COUNTER__
> 
> $ gcc -E test1.c test2.c
> # 1 "test1.c"
> # 1 "<built-in>"
> # 1 "<command-line>"
> # 1 "/usr/include/stdc-predef.h" 1 3 4
> # 1 "<command-line>" 2
> # 1 "test1.c"
> 0
> 1
> # 1 "test2.c"
> # 1 "<built-in>"
> # 1 "<command-line>"
> # 1 "/usr/include/stdc-predef.h" 1 3 4
> # 1 "<command-line>" 2
> # 1 "test2.c"
> 0
> 1
> 
> Notice that the second file starts with __COUNTER__ at 0 again.
> 
> The counter *should* keep counting through include files, but needs to
> reset before starting a new top-level compile.
> 
> - Josh Triplett
> --

Yes, indeed.
Thanks for bringing to my attention.

Here is a new version of the patch taking care of that.


Luc

---
Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.

This macro expands to sequential integral values starting from 0,
and this for each top-level source file.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 ident-list.h                       |  1 +
 pre-process.c                      |  4 ++++
 validation/preprocessor/counter1.c | 12 ++++++++++++
 validation/preprocessor/counter2.c | 14 ++++++++++++++
 validation/preprocessor/counter2.h |  1 +
 validation/preprocessor/counter3.c | 13 +++++++++++++
 6 files changed, 45 insertions(+)
 create mode 100644 validation/preprocessor/counter1.c
 create mode 100644 validation/preprocessor/counter2.c
 create mode 100644 validation/preprocessor/counter2.h
 create mode 100644 validation/preprocessor/counter3.c

diff --git a/ident-list.h b/ident-list.h
index d5a145f8..b65b667d 100644
--- a/ident-list.h
+++ b/ident-list.h
@@ -108,6 +108,7 @@ __IDENT(__TIME___ident, "__TIME__", 0);
 __IDENT(__func___ident, "__func__", 0);
 __IDENT(__FUNCTION___ident, "__FUNCTION__", 0);
 __IDENT(__PRETTY_FUNCTION___ident, "__PRETTY_FUNCTION__", 0);
+__IDENT(__COUNTER___ident, "__COUNTER__", 0);
 
 /* Sparse commands */
 IDENT_RESERVED(__context__);
diff --git a/pre-process.c b/pre-process.c
index 1aa3d2c4..601e0f26 100644
--- a/pre-process.c
+++ b/pre-process.c
@@ -45,6 +45,7 @@
 #include "scope.h"
 
 static int false_nesting = 0;
+static int counter_macro;		// __COUNTER__ expansion
 
 #define INCLUDEPATHS 300
 const char *includepath[INCLUDEPATHS+1] = {
@@ -181,6 +182,8 @@ static int expand_one_symbol(struct token **list)
 			time(&t);
 		strftime(buffer, 9, "%T", localtime(&t));
 		replace_with_string(token, buffer);
+	} else if (token->ident == &__COUNTER___ident) {
+		replace_with_integer(token, counter_macro++);
 	}
 	return 1;
 }
@@ -1882,6 +1885,7 @@ static void init_preprocessor(void)
 		sym->normal = 0;
 	}
 
+	counter_macro = 0;
 }
 
 static void handle_preprocessor_line(struct stream *stream, struct token **line, struct token *start)
diff --git a/validation/preprocessor/counter1.c b/validation/preprocessor/counter1.c
new file mode 100644
index 00000000..98187ee6
--- /dev/null
+++ b/validation/preprocessor/counter1.c
@@ -0,0 +1,12 @@
+__COUNTER__
+__COUNTER__
+/*
+ * check-name: __COUNTER__ #1
+ * check-command: sparse -E $file
+ *
+ * check-output-start
+
+0
+1
+ * check-output-end
+ */
diff --git a/validation/preprocessor/counter2.c b/validation/preprocessor/counter2.c
new file mode 100644
index 00000000..9883b682
--- /dev/null
+++ b/validation/preprocessor/counter2.c
@@ -0,0 +1,14 @@
+__FILE__ __COUNTER__
+#include <counter2.h>
+__FILE__ __COUNTER__
+/*
+ * check-name: __COUNTER__ #2
+ * check-command: sparse -Ipreprocessor -E $file
+ *
+ * check-output-start
+
+"preprocessor/counter2.c" 0
+"preprocessor/counter2.h" 1
+"preprocessor/counter2.c" 2
+ * check-output-end
+ */
diff --git a/validation/preprocessor/counter2.h b/validation/preprocessor/counter2.h
new file mode 100644
index 00000000..447b70ab
--- /dev/null
+++ b/validation/preprocessor/counter2.h
@@ -0,0 +1 @@
+__FILE__ __COUNTER__
diff --git a/validation/preprocessor/counter3.c b/validation/preprocessor/counter3.c
new file mode 100644
index 00000000..1449b2d1
--- /dev/null
+++ b/validation/preprocessor/counter3.c
@@ -0,0 +1,13 @@
+/*
+ * check-name: __COUNTER__ #3
+ * check-command: sparse -Ipreprocessor -E preprocessor/counter1.c preprocessor/counter2.c
+ *
+ * check-output-start
+
+0
+1
+"preprocessor/counter2.c" 0
+"preprocessor/counter2.h" 1
+"preprocessor/counter2.c" 2
+ * check-output-end
+ */

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro
  2015-01-23 23:59       ` Luc Van Oostenryck
@ 2015-01-24  1:29         ` Josh Triplett
  2015-01-24 11:27           ` Luc Van Oostenryck
  2015-01-25 20:12         ` Christian Borntraeger
  2015-02-02  5:17         ` Christopher Li
  2 siblings, 1 reply; 20+ messages in thread
From: Josh Triplett @ 2015-01-24  1:29 UTC (permalink / raw)
  To: Luc Van Oostenryck
  Cc: Christopher Li, Christian Borntraeger, Linus Torvalds,
	Jason J. Herne, Linux-Sparse

On Sat, Jan 24, 2015 at 12:59:35AM +0100, Luc Van Oostenryck wrote:
> Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
> 
> This macro expands to sequential integral values starting from 0,
> and this for each top-level source file.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

counter3.c seems like a bit of an abuse of the test suite framework, but
I don't have a better suggestion.

Reviewed-by: Josh Triplett <josh@joshtriplett.org>

>  ident-list.h                       |  1 +
>  pre-process.c                      |  4 ++++
>  validation/preprocessor/counter1.c | 12 ++++++++++++
>  validation/preprocessor/counter2.c | 14 ++++++++++++++
>  validation/preprocessor/counter2.h |  1 +
>  validation/preprocessor/counter3.c | 13 +++++++++++++
>  6 files changed, 45 insertions(+)
>  create mode 100644 validation/preprocessor/counter1.c
>  create mode 100644 validation/preprocessor/counter2.c
>  create mode 100644 validation/preprocessor/counter2.h
>  create mode 100644 validation/preprocessor/counter3.c
> 
> diff --git a/ident-list.h b/ident-list.h
> index d5a145f8..b65b667d 100644
> --- a/ident-list.h
> +++ b/ident-list.h
> @@ -108,6 +108,7 @@ __IDENT(__TIME___ident, "__TIME__", 0);
>  __IDENT(__func___ident, "__func__", 0);
>  __IDENT(__FUNCTION___ident, "__FUNCTION__", 0);
>  __IDENT(__PRETTY_FUNCTION___ident, "__PRETTY_FUNCTION__", 0);
> +__IDENT(__COUNTER___ident, "__COUNTER__", 0);
>  
>  /* Sparse commands */
>  IDENT_RESERVED(__context__);
> diff --git a/pre-process.c b/pre-process.c
> index 1aa3d2c4..601e0f26 100644
> --- a/pre-process.c
> +++ b/pre-process.c
> @@ -45,6 +45,7 @@
>  #include "scope.h"
>  
>  static int false_nesting = 0;
> +static int counter_macro;		// __COUNTER__ expansion
>  
>  #define INCLUDEPATHS 300
>  const char *includepath[INCLUDEPATHS+1] = {
> @@ -181,6 +182,8 @@ static int expand_one_symbol(struct token **list)
>  			time(&t);
>  		strftime(buffer, 9, "%T", localtime(&t));
>  		replace_with_string(token, buffer);
> +	} else if (token->ident == &__COUNTER___ident) {
> +		replace_with_integer(token, counter_macro++);
>  	}
>  	return 1;
>  }
> @@ -1882,6 +1885,7 @@ static void init_preprocessor(void)
>  		sym->normal = 0;
>  	}
>  
> +	counter_macro = 0;
>  }
>  
>  static void handle_preprocessor_line(struct stream *stream, struct token **line, struct token *start)
> diff --git a/validation/preprocessor/counter1.c b/validation/preprocessor/counter1.c
> new file mode 100644
> index 00000000..98187ee6
> --- /dev/null
> +++ b/validation/preprocessor/counter1.c
> @@ -0,0 +1,12 @@
> +__COUNTER__
> +__COUNTER__
> +/*
> + * check-name: __COUNTER__ #1
> + * check-command: sparse -E $file
> + *
> + * check-output-start
> +
> +0
> +1
> + * check-output-end
> + */
> diff --git a/validation/preprocessor/counter2.c b/validation/preprocessor/counter2.c
> new file mode 100644
> index 00000000..9883b682
> --- /dev/null
> +++ b/validation/preprocessor/counter2.c
> @@ -0,0 +1,14 @@
> +__FILE__ __COUNTER__
> +#include <counter2.h>
> +__FILE__ __COUNTER__
> +/*
> + * check-name: __COUNTER__ #2
> + * check-command: sparse -Ipreprocessor -E $file
> + *
> + * check-output-start
> +
> +"preprocessor/counter2.c" 0
> +"preprocessor/counter2.h" 1
> +"preprocessor/counter2.c" 2
> + * check-output-end
> + */
> diff --git a/validation/preprocessor/counter2.h b/validation/preprocessor/counter2.h
> new file mode 100644
> index 00000000..447b70ab
> --- /dev/null
> +++ b/validation/preprocessor/counter2.h
> @@ -0,0 +1 @@
> +__FILE__ __COUNTER__
> diff --git a/validation/preprocessor/counter3.c b/validation/preprocessor/counter3.c
> new file mode 100644
> index 00000000..1449b2d1
> --- /dev/null
> +++ b/validation/preprocessor/counter3.c
> @@ -0,0 +1,13 @@
> +/*
> + * check-name: __COUNTER__ #3
> + * check-command: sparse -Ipreprocessor -E preprocessor/counter1.c preprocessor/counter2.c
> + *
> + * check-output-start
> +
> +0
> +1
> +"preprocessor/counter2.c" 0
> +"preprocessor/counter2.h" 1
> +"preprocessor/counter2.c" 2
> + * check-output-end
> + */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro
  2015-01-24  1:29         ` Josh Triplett
@ 2015-01-24 11:27           ` Luc Van Oostenryck
  2015-01-24 20:19             ` Josh Triplett
  0 siblings, 1 reply; 20+ messages in thread
From: Luc Van Oostenryck @ 2015-01-24 11:27 UTC (permalink / raw)
  To: Josh Triplett; +Cc: Christopher Li, Sam Ravnborg, Linux-Sparse

On Fri, Jan 23, 2015 at 05:29:58PM -0800, Josh Triplett wrote:
> On Sat, Jan 24, 2015 at 12:59:35AM +0100, Luc Van Oostenryck wrote:
> > Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
> > 
> > This macro expands to sequential integral values starting from 0,
> > and this for each top-level source file.
> > 
> > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> 
> counter3.c seems like a bit of an abuse of the test suite framework, but
> I don't have a better suggestion.
> 
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>

Yes, I know ...

Would the following change to the test-suite (introducing tags to separate input sections)
and the corresponding test be more OK ?


Luc

diff --git a/validation/test-suite b/validation/test-suite
index df5a7c60..97d4dd40 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -102,6 +102,15 @@ do_test()
 	fi
 	test_name=$last_result
 
+	# grab the input sections
+	input_nr=1
+	while grep -q "input-file-$input_nr-start" "$file"; do
+		sed -n "/input-file-$input_nr-start/,/input-file-$input_nr-end/p" "$file" \
+			| grep -v input-file > "$file".input$input_nr
+		eval "file$input_nr=$file.input$input_nr"
+		input_nr=$(($input_nr + 1))
+	done
+
 	# does the test provide a specific command ?
 	cmd=`eval echo $default_path/$default_cmd`
 	get_value "check-command" $file
diff --git a/Documentation/test-suite b/Documentation/test-suite
index 6c4f24f6..34b38696 100644
--- a/Documentation/test-suite
+++ b/Documentation/test-suite
@@ -32,6 +32,10 @@ check-output-start / check-output-end (optional)
 check-known-to-fail (optional)
 	Mark the test as being known to fail.
 
+input-file-1-start / input-file-1-end, / input-file-2-start / ... (optional)
+	The input files of check-command lies between those tags.
+	It's only needed when the test needs several distincts input files.
+
 
 	Using test-suite
 	~~~~~~~~~~~~~~~~
@@ -58,6 +62,9 @@ name:
 cmd:
 	check-command value. If no cmd is provided, it defaults to
 	"sparse $file".
+	If the "input-file-1-start/..." tags are used those files are to be
+	referenced with "$file1", ... and the command need to be something like:
+	"sparse $file1 $file2"
 
 The output of the test-suite format command can be redirected into the
 test case to create a test-suite formated file.
diff --git a/validation/preprocessor/counter3.c b/validation/preprocessor/counter3.c
index e69de29b..76635e82 100644
--- a/validation/preprocessor/counter3.c
+++ b/validation/preprocessor/counter3.c
@@ -0,0 +1,20 @@
+/* input-file-1-start */
+__COUNTER__
+__COUNTER__
+/* input-file-1-end */
+
+/* input-file-2-start */
+__COUNTER__
+/* input-file-2-end */
+
+/*
+ * check-name: __COUNTER__ #3
+ * check-command: sparse -E $file1 $file2
+ *
+ * check-output-start
+
+0
+1
+0
+ * check-output-end
+ */

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro
  2015-01-24 11:27           ` Luc Van Oostenryck
@ 2015-01-24 20:19             ` Josh Triplett
  2015-01-24 20:39               ` Luc Van Oostenryck
  0 siblings, 1 reply; 20+ messages in thread
From: Josh Triplett @ 2015-01-24 20:19 UTC (permalink / raw)
  To: Luc Van Oostenryck; +Cc: Christopher Li, Sam Ravnborg, Linux-Sparse

On Sat, Jan 24, 2015 at 12:27:06PM +0100, Luc Van Oostenryck wrote:
> On Fri, Jan 23, 2015 at 05:29:58PM -0800, Josh Triplett wrote:
> > On Sat, Jan 24, 2015 at 12:59:35AM +0100, Luc Van Oostenryck wrote:
> > > Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
> > > 
> > > This macro expands to sequential integral values starting from 0,
> > > and this for each top-level source file.
> > > 
> > > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> > 
> > counter3.c seems like a bit of an abuse of the test suite framework, but
> > I don't have a better suggestion.
> > 
> > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> 
> Yes, I know ...
> 
> Would the following change to the test-suite (introducing tags to separate input sections)
> and the corresponding test be more OK ?

Interesting idea!  That would also allow consolidating tests that
require include files into a single test file, if it's possible to
#include "$file1"; for instance, pragma-once.c could avoid recursing if
the test fails.

I'll leave it to others to decide if this seems like a direction the
test suite should expand to cover, or if for this single case counter3.c
should just include other tests as your previous patch did.

- Josh Triplett

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro
  2015-01-24 20:19             ` Josh Triplett
@ 2015-01-24 20:39               ` Luc Van Oostenryck
  0 siblings, 0 replies; 20+ messages in thread
From: Luc Van Oostenryck @ 2015-01-24 20:39 UTC (permalink / raw)
  To: Josh Triplett; +Cc: Christopher Li, Sam Ravnborg, Linux-Sparse

On Sat, Jan 24, 2015 at 12:19:50PM -0800, Josh Triplett wrote:
> On Sat, Jan 24, 2015 at 12:27:06PM +0100, Luc Van Oostenryck wrote:
> > On Fri, Jan 23, 2015 at 05:29:58PM -0800, Josh Triplett wrote:
> > > On Sat, Jan 24, 2015 at 12:59:35AM +0100, Luc Van Oostenryck wrote:
> > > > Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
> > > > 
> > > > This macro expands to sequential integral values starting from 0,
> > > > and this for each top-level source file.
> > > > 
> > > > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> > > 
> > > counter3.c seems like a bit of an abuse of the test suite framework, but
> > > I don't have a better suggestion.
> > > 
> > > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > 
> > Yes, I know ...
> > 
> > Would the following change to the test-suite (introducing tags to separate input sections)
> > and the corresponding test be more OK ?
> 
> Interesting idea!  That would also allow consolidating tests that
> require include files into a single test file, if it's possible to
> #include "$file1"; for instance, pragma-once.c could avoid recursing if
> the test fails.
> 
> I'll leave it to others to decide if this seems like a direction the
> test suite should expand to cover, or if for this single case counter3.c
> should just include other tests as your previous patch did.
> 
> - Josh Triplett
> --

Yes, it's fairly easy to add. In fact ... yesterday I had a version that
did that but I removed it because I found it a bit hackish and it was
not needed.


Luc

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro
  2015-01-23 23:59       ` Luc Van Oostenryck
  2015-01-24  1:29         ` Josh Triplett
@ 2015-01-25 20:12         ` Christian Borntraeger
  2015-01-28 10:08           ` Christian Borntraeger
  2015-02-02  5:17         ` Christopher Li
  2 siblings, 1 reply; 20+ messages in thread
From: Christian Borntraeger @ 2015-01-25 20:12 UTC (permalink / raw)
  To: Luc Van Oostenryck, josh
  Cc: Christopher Li, Linus Torvalds, Jason J. Herne, Linux-Sparse

Am 24.01.2015 um 00:59 schrieb Luc Van Oostenryck:
[...] 
> Yes, indeed.
> Thanks for bringing to my attention.
> 
> Here is a new version of the patch taking care of that.
> 
> 
> Luc
> 
> ---
> Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
> 
> This macro expands to sequential integral values starting from 0,
> and this for each top-level source file.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>

> ---
>  ident-list.h                       |  1 +
>  pre-process.c                      |  4 ++++
>  validation/preprocessor/counter1.c | 12 ++++++++++++
>  validation/preprocessor/counter2.c | 14 ++++++++++++++
>  validation/preprocessor/counter2.h |  1 +
>  validation/preprocessor/counter3.c | 13 +++++++++++++
>  6 files changed, 45 insertions(+)
>  create mode 100644 validation/preprocessor/counter1.c
>  create mode 100644 validation/preprocessor/counter2.c
>  create mode 100644 validation/preprocessor/counter2.h
>  create mode 100644 validation/preprocessor/counter3.c
> 
> diff --git a/ident-list.h b/ident-list.h
> index d5a145f8..b65b667d 100644
> --- a/ident-list.h
> +++ b/ident-list.h
> @@ -108,6 +108,7 @@ __IDENT(__TIME___ident, "__TIME__", 0);
>  __IDENT(__func___ident, "__func__", 0);
>  __IDENT(__FUNCTION___ident, "__FUNCTION__", 0);
>  __IDENT(__PRETTY_FUNCTION___ident, "__PRETTY_FUNCTION__", 0);
> +__IDENT(__COUNTER___ident, "__COUNTER__", 0);
> 
>  /* Sparse commands */
>  IDENT_RESERVED(__context__);
> diff --git a/pre-process.c b/pre-process.c
> index 1aa3d2c4..601e0f26 100644
> --- a/pre-process.c
> +++ b/pre-process.c
> @@ -45,6 +45,7 @@
>  #include "scope.h"
> 
>  static int false_nesting = 0;
> +static int counter_macro;		// __COUNTER__ expansion
> 
>  #define INCLUDEPATHS 300
>  const char *includepath[INCLUDEPATHS+1] = {
> @@ -181,6 +182,8 @@ static int expand_one_symbol(struct token **list)
>  			time(&t);
>  		strftime(buffer, 9, "%T", localtime(&t));
>  		replace_with_string(token, buffer);
> +	} else if (token->ident == &__COUNTER___ident) {
> +		replace_with_integer(token, counter_macro++);
>  	}
>  	return 1;
>  }
> @@ -1882,6 +1885,7 @@ static void init_preprocessor(void)
>  		sym->normal = 0;
>  	}
> 
> +	counter_macro = 0;
>  }
> 
>  static void handle_preprocessor_line(struct stream *stream, struct token **line, struct token *start)
> diff --git a/validation/preprocessor/counter1.c b/validation/preprocessor/counter1.c
> new file mode 100644
> index 00000000..98187ee6
> --- /dev/null
> +++ b/validation/preprocessor/counter1.c
> @@ -0,0 +1,12 @@
> +__COUNTER__
> +__COUNTER__
> +/*
> + * check-name: __COUNTER__ #1
> + * check-command: sparse -E $file
> + *
> + * check-output-start
> +
> +0
> +1
> + * check-output-end
> + */
> diff --git a/validation/preprocessor/counter2.c b/validation/preprocessor/counter2.c
> new file mode 100644
> index 00000000..9883b682
> --- /dev/null
> +++ b/validation/preprocessor/counter2.c
> @@ -0,0 +1,14 @@
> +__FILE__ __COUNTER__
> +#include <counter2.h>
> +__FILE__ __COUNTER__
> +/*
> + * check-name: __COUNTER__ #2
> + * check-command: sparse -Ipreprocessor -E $file
> + *
> + * check-output-start
> +
> +"preprocessor/counter2.c" 0
> +"preprocessor/counter2.h" 1
> +"preprocessor/counter2.c" 2
> + * check-output-end
> + */
> diff --git a/validation/preprocessor/counter2.h b/validation/preprocessor/counter2.h
> new file mode 100644
> index 00000000..447b70ab
> --- /dev/null
> +++ b/validation/preprocessor/counter2.h
> @@ -0,0 +1 @@
> +__FILE__ __COUNTER__
> diff --git a/validation/preprocessor/counter3.c b/validation/preprocessor/counter3.c
> new file mode 100644
> index 00000000..1449b2d1
> --- /dev/null
> +++ b/validation/preprocessor/counter3.c
> @@ -0,0 +1,13 @@
> +/*
> + * check-name: __COUNTER__ #3
> + * check-command: sparse -Ipreprocessor -E preprocessor/counter1.c preprocessor/counter2.c
> + *
> + * check-output-start
> +
> +0
> +1
> +"preprocessor/counter2.c" 0
> +"preprocessor/counter2.h" 1
> +"preprocessor/counter2.c" 2
> + * check-output-end
> + */
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro
  2015-01-25 20:12         ` Christian Borntraeger
@ 2015-01-28 10:08           ` Christian Borntraeger
  0 siblings, 0 replies; 20+ messages in thread
From: Christian Borntraeger @ 2015-01-28 10:08 UTC (permalink / raw)
  To: Luc Van Oostenryck, josh
  Cc: Christopher Li, Linus Torvalds, Jason J. Herne, Linux-Sparse

Am 25.01.2015 um 21:12 schrieb Christian Borntraeger:
> Am 24.01.2015 um 00:59 schrieb Luc Van Oostenryck:
> [...] 
>> Yes, indeed.
>> Thanks for bringing to my attention.
>>
>> Here is a new version of the patch taking care of that.
>>
>>
>> Luc
>>
>> ---
>> Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
>>
>> This macro expands to sequential integral values starting from 0,
>> and this for each top-level source file.
>>
>> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> 
> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>

Christoper,
can you apply that fix as well?

Thans

> 
>> ---
>>  ident-list.h                       |  1 +
>>  pre-process.c                      |  4 ++++
>>  validation/preprocessor/counter1.c | 12 ++++++++++++
>>  validation/preprocessor/counter2.c | 14 ++++++++++++++
>>  validation/preprocessor/counter2.h |  1 +
>>  validation/preprocessor/counter3.c | 13 +++++++++++++
>>  6 files changed, 45 insertions(+)
>>  create mode 100644 validation/preprocessor/counter1.c
>>  create mode 100644 validation/preprocessor/counter2.c
>>  create mode 100644 validation/preprocessor/counter2.h
>>  create mode 100644 validation/preprocessor/counter3.c
>>
>> diff --git a/ident-list.h b/ident-list.h
>> index d5a145f8..b65b667d 100644
>> --- a/ident-list.h
>> +++ b/ident-list.h
>> @@ -108,6 +108,7 @@ __IDENT(__TIME___ident, "__TIME__", 0);
>>  __IDENT(__func___ident, "__func__", 0);
>>  __IDENT(__FUNCTION___ident, "__FUNCTION__", 0);
>>  __IDENT(__PRETTY_FUNCTION___ident, "__PRETTY_FUNCTION__", 0);
>> +__IDENT(__COUNTER___ident, "__COUNTER__", 0);
>>
>>  /* Sparse commands */
>>  IDENT_RESERVED(__context__);
>> diff --git a/pre-process.c b/pre-process.c
>> index 1aa3d2c4..601e0f26 100644
>> --- a/pre-process.c
>> +++ b/pre-process.c
>> @@ -45,6 +45,7 @@
>>  #include "scope.h"
>>
>>  static int false_nesting = 0;
>> +static int counter_macro;		// __COUNTER__ expansion
>>
>>  #define INCLUDEPATHS 300
>>  const char *includepath[INCLUDEPATHS+1] = {
>> @@ -181,6 +182,8 @@ static int expand_one_symbol(struct token **list)
>>  			time(&t);
>>  		strftime(buffer, 9, "%T", localtime(&t));
>>  		replace_with_string(token, buffer);
>> +	} else if (token->ident == &__COUNTER___ident) {
>> +		replace_with_integer(token, counter_macro++);
>>  	}
>>  	return 1;
>>  }
>> @@ -1882,6 +1885,7 @@ static void init_preprocessor(void)
>>  		sym->normal = 0;
>>  	}
>>
>> +	counter_macro = 0;
>>  }
>>
>>  static void handle_preprocessor_line(struct stream *stream, struct token **line, struct token *start)
>> diff --git a/validation/preprocessor/counter1.c b/validation/preprocessor/counter1.c
>> new file mode 100644
>> index 00000000..98187ee6
>> --- /dev/null
>> +++ b/validation/preprocessor/counter1.c
>> @@ -0,0 +1,12 @@
>> +__COUNTER__
>> +__COUNTER__
>> +/*
>> + * check-name: __COUNTER__ #1
>> + * check-command: sparse -E $file
>> + *
>> + * check-output-start
>> +
>> +0
>> +1
>> + * check-output-end
>> + */
>> diff --git a/validation/preprocessor/counter2.c b/validation/preprocessor/counter2.c
>> new file mode 100644
>> index 00000000..9883b682
>> --- /dev/null
>> +++ b/validation/preprocessor/counter2.c
>> @@ -0,0 +1,14 @@
>> +__FILE__ __COUNTER__
>> +#include <counter2.h>
>> +__FILE__ __COUNTER__
>> +/*
>> + * check-name: __COUNTER__ #2
>> + * check-command: sparse -Ipreprocessor -E $file
>> + *
>> + * check-output-start
>> +
>> +"preprocessor/counter2.c" 0
>> +"preprocessor/counter2.h" 1
>> +"preprocessor/counter2.c" 2
>> + * check-output-end
>> + */
>> diff --git a/validation/preprocessor/counter2.h b/validation/preprocessor/counter2.h
>> new file mode 100644
>> index 00000000..447b70ab
>> --- /dev/null
>> +++ b/validation/preprocessor/counter2.h
>> @@ -0,0 +1 @@
>> +__FILE__ __COUNTER__
>> diff --git a/validation/preprocessor/counter3.c b/validation/preprocessor/counter3.c
>> new file mode 100644
>> index 00000000..1449b2d1
>> --- /dev/null
>> +++ b/validation/preprocessor/counter3.c
>> @@ -0,0 +1,13 @@
>> +/*
>> + * check-name: __COUNTER__ #3
>> + * check-command: sparse -Ipreprocessor -E preprocessor/counter1.c preprocessor/counter2.c
>> + *
>> + * check-output-start
>> +
>> +0
>> +1
>> +"preprocessor/counter2.c" 0
>> +"preprocessor/counter2.h" 1
>> +"preprocessor/counter2.c" 2
>> + * check-output-end
>> + */
>>
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro
  2015-01-23 23:59       ` Luc Van Oostenryck
  2015-01-24  1:29         ` Josh Triplett
  2015-01-25 20:12         ` Christian Borntraeger
@ 2015-02-02  5:17         ` Christopher Li
  2015-02-04  2:38           ` Luc Van Oostenryck
                             ` (4 more replies)
  2 siblings, 5 replies; 20+ messages in thread
From: Christopher Li @ 2015-02-02  5:17 UTC (permalink / raw)
  To: Luc Van Oostenryck
  Cc: Josh Triplett, Christian Borntraeger, Linus Torvalds,
	Jason J. Herne, Linux-Sparse, Sam Ravnborg

On Fri, Jan 23, 2015 at 3:59 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Thanks for bringing to my attention.
>
> Here is a new version of the patch taking care of that.
>

I like Sam's commit message better. So I merge a version using his
commit message.

The branch for review is here:
https://git.kernel.org/cgit/devel/sparse/chrisl/sparse.git/log/?h=review-counter

If there is no objects, I will merge it to master.

Chris

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro
  2015-02-02  5:17         ` Christopher Li
@ 2015-02-04  2:38           ` Luc Van Oostenryck
  2015-02-12 20:16             ` Christopher Li
  2015-02-04  2:46           ` [PATCH 1/3] test-suite: add support for tests case involving several input files Luc Van Oostenryck
                             ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Luc Van Oostenryck @ 2015-02-04  2:38 UTC (permalink / raw)
  To: Christopher Li
  Cc: Josh Triplett, Christian Borntraeger, Jason J. Herne,
	Linux-Sparse, Sam Ravnborg

On Sun, Feb 01, 2015 at 09:17:38PM -0800, Christopher Li wrote:
> On Fri, Jan 23, 2015 at 3:59 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > Thanks for bringing to my attention.
> >
> > Here is a new version of the patch taking care of that.
> >
> 
> I like Sam's commit message better. So I merge a version using his
> commit message.
> 
> The branch for review is here:
> https://git.kernel.org/cgit/devel/sparse/chrisl/sparse.git/log/?h=review-counter
> 
> If there is no objects, I will merge it to master.
> 
> Chris
> --

Fine for me but Josh had a remark about the third test file (validation/preprocessor/counter3.c)
abusing a bit the test framework and I shared his feeling.

Here is a patch serie fixing this abuse by extending the test framework.


1/3) test-suite: add support for tests case involving several input files
2/3) test-suite: allow filename expansion of the input sections
3/3) test-suite: consolidate tests that require include files into single test files

 Documentation/test-suite                 | 11 +++++++++++
 validation/.gitignore                    |  1 +
 validation/pragma-once.c                 | 13 ++++++++++++-
 validation/preprocessor/counter2.c       | 16 +++++++++++-----
 validation/preprocessor/counter2.h       |  1 -
 validation/preprocessor/counter3.c       | 15 +++++++++++----
 validation/preprocessor/preprocessor20.c | 18 +++++++++++++++---
 validation/preprocessor/preprocessor20.h |  6 ------
 validation/test-suite                    | 11 +++++++++++
 validation/test-suite-file-expansion.c   | 22 ++++++++++++++++++++++
 10 files changed, 94 insertions(+), 20 deletions(-)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/3] test-suite: add support for tests case involving several input files
  2015-02-02  5:17         ` Christopher Li
  2015-02-04  2:38           ` Luc Van Oostenryck
@ 2015-02-04  2:46           ` Luc Van Oostenryck
  2015-02-06 15:02             ` Christopher Li
  2015-02-04  2:49           ` [PATCH 2/3] test-suite: allow filename expansion of the input sections Luc Van Oostenryck
                             ` (2 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Luc Van Oostenryck @ 2015-02-04  2:46 UTC (permalink / raw)
  To: Christopher Li
  Cc: Josh Triplett, Christian Borntraeger, Jason J. Herne,
	Linux-Sparse, Sam Ravnborg


These input files, instead of being stored in real files and thus polluting/confusing
the test framework are now placed in a serie of sections inside an unique test file.
These sections are delimited by new tags: input-file-1-start / input-file-1-end, / input-file-2-start/ ...

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Inspired-by: Josh Triplett <josh@joshtriplett.org>

---
 Documentation/test-suite           |  7 +++++++
 validation/.gitignore              |  1 +
 validation/preprocessor/counter3.c | 15 +++++++++++----
 validation/test-suite              |  9 +++++++++
 4 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/Documentation/test-suite b/Documentation/test-suite
index 6c4f24f6..e2ce7003 100644
--- a/Documentation/test-suite
+++ b/Documentation/test-suite
@@ -32,6 +32,10 @@ check-output-start / check-output-end (optional)
 check-known-to-fail (optional)
 	Mark the test as being known to fail.
 
+input-file-1-start / input-file-1-end, / input-file-2-start/ ... (optional)
+	The input files of check-command lies between those two tags.
+	It's only needed when a test requires several files.
+
 
 	Using test-suite
 	~~~~~~~~~~~~~~~~
@@ -58,6 +62,9 @@ name:
 cmd:
 	check-command value. If no cmd is provided, it defaults to
 	"sparse $file".
+	If the "input-file-1-start/..." tags are used those files are to be
+	referenced with "$file1", ... and the command need to be something like:
+	"sparse $file1 $file2"
 
 The output of the test-suite format command can be redirected into the
 test case to create a test-suite formated file.
diff --git a/validation/.gitignore b/validation/.gitignore
index 77276ba4..2a5f114b 100644
--- a/validation/.gitignore
+++ b/validation/.gitignore
@@ -2,3 +2,4 @@
 *.diff
 *.got
 *.expected
+*.input[1-9]
diff --git a/validation/preprocessor/counter3.c b/validation/preprocessor/counter3.c
index 1449b2d1..76635e82 100644
--- a/validation/preprocessor/counter3.c
+++ b/validation/preprocessor/counter3.c
@@ -1,13 +1,20 @@
+/* input-file-1-start */
+__COUNTER__
+__COUNTER__
+/* input-file-1-end */
+
+/* input-file-2-start */
+__COUNTER__
+/* input-file-2-end */
+
 /*
  * check-name: __COUNTER__ #3
- * check-command: sparse -Ipreprocessor -E preprocessor/counter1.c preprocessor/counter2.c
+ * check-command: sparse -E $file1 $file2
  *
  * check-output-start
 
 0
 1
-"preprocessor/counter2.c" 0
-"preprocessor/counter2.h" 1
-"preprocessor/counter2.c" 2
+0
  * check-output-end
  */
diff --git a/validation/test-suite b/validation/test-suite
index df5a7c60..97d4dd40 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -102,6 +102,15 @@ do_test()
 	fi
 	test_name=$last_result
 
+	# grab the input sections
+	input_nr=1
+	while grep -q "input-file-$input_nr-start" "$file"; do
+		sed -n "/input-file-$input_nr-start/,/input-file-$input_nr-end/p" "$file" \
+			| grep -v input-file > "$file".input$input_nr
+		eval "file$input_nr=$file.input$input_nr"
+		input_nr=$(($input_nr + 1))
+	done
+
 	# does the test provide a specific command ?
 	cmd=`eval echo $default_path/$default_cmd`
 	get_value "check-command" $file
-- 
2.2.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/3] test-suite: allow filename expansion of the input sections
  2015-02-02  5:17         ` Christopher Li
  2015-02-04  2:38           ` Luc Van Oostenryck
  2015-02-04  2:46           ` [PATCH 1/3] test-suite: add support for tests case involving several input files Luc Van Oostenryck
@ 2015-02-04  2:49           ` Luc Van Oostenryck
  2015-02-04  2:51           ` [PATCH 3/3] test-suite: consolidate tests that require include files into single test files Luc Van Oostenryck
  2015-02-04  3:11           ` [PATCH 2/3] test-suite: allow filename expansion of the input sections Luc Van Oostenryck
  4 siblings, 0 replies; 20+ messages in thread
From: Luc Van Oostenryck @ 2015-02-04  2:49 UTC (permalink / raw)
  To: Christopher Li
  Cc: Josh Triplett, Christian Borntraeger, Linus Torvalds,
	Jason J. Herne, Linux-Sparse, Sam Ravnborg

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Suggested-by: Josh Triplett <josh@joshtriplett.org>
---
 Documentation/test-suite               |  4 ++++
 validation/test-suite                  |  2 ++
 validation/test-suite-file-expansion.c | 22 ++++++++++++++++++++++
 3 files changed, 28 insertions(+)
 create mode 100644 validation/test-suite-file-expansion.c

diff --git a/Documentation/test-suite b/Documentation/test-suite
index e2ce7003..86bee335 100644
--- a/Documentation/test-suite
+++ b/Documentation/test-suite
@@ -35,6 +35,10 @@ check-known-to-fail (optional)
 input-file-1-start / input-file-1-end, / input-file-2-start/ ... (optional)
 	The input files of check-command lies between those two tags.
 	It's only needed when a test requires several files.
+	The '$file1', '$file2', ... strings are special. They will be expanded
+	at run time to the name of each of these files. These strings can be used
+	inside these input sections themselves, the check-command or the
+	check-output section.
 
 
 	Using test-suite
diff --git a/validation/test-suite b/validation/test-suite
index 97d4dd40..8b0cd5e4 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -106,6 +106,7 @@ do_test()
 	input_nr=1
 	while grep -q "input-file-$input_nr-start" "$file"; do
 		sed -n "/input-file-$input_nr-start/,/input-file-$input_nr-end/p" "$file" \
+			| sed "s:\\\$file\\([1-9]\\):$(basename $file).input\\1:" \
 			| grep -v input-file > "$file".input$input_nr
 		eval "file$input_nr=$file.input$input_nr"
 		input_nr=$(($input_nr + 1))
@@ -136,6 +137,7 @@ do_test()
 
 	# grab the expected output
 	sed -n '/check-output-start/,/check-output-end/p' $file \
+		| sed "s:\\\$file\\([1-9]\\):$file.input\\1:" \
 		| grep -v check-output > "$file".output.expected
 	sed -n '/check-error-start/,/check-error-end/p' $file \
 		| grep -v check-error > "$file".error.expected
diff --git a/validation/test-suite-file-expansion.c b/validation/test-suite-file-expansion.c
new file mode 100644
index 00000000..fc4ff8d5
--- /dev/null
+++ b/validation/test-suite-file-expansion.c
@@ -0,0 +1,22 @@
+/* input-file-1-start */
+const char file = __FILE__;
+/* input-file-1-end */
+
+/* input-file-2-start */
+int val = 0;
+#include "$file1"
+const char file = __FILE__;
+/* input-file-2-end */
+
+/*
+ * check-name: test-suite $file* expansion
+ * check-command: sparse -E $file1 $file2
+ *
+ * check-output-start
+
+const char file = "$file1";
+int val = 0;
+const char file = "$file1";
+const char file = "$file2";
+ * check-output-end
+ */
-- 
2.2.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/3] test-suite: consolidate tests that require include files into single test files
  2015-02-02  5:17         ` Christopher Li
                             ` (2 preceding siblings ...)
  2015-02-04  2:49           ` [PATCH 2/3] test-suite: allow filename expansion of the input sections Luc Van Oostenryck
@ 2015-02-04  2:51           ` Luc Van Oostenryck
  2015-02-04  3:11           ` [PATCH 2/3] test-suite: allow filename expansion of the input sections Luc Van Oostenryck
  4 siblings, 0 replies; 20+ messages in thread
From: Luc Van Oostenryck @ 2015-02-04  2:51 UTC (permalink / raw)
  To: Christopher Li
  Cc: Josh Triplett, Christian Borntraeger, Jason J. Herne,
	Linux-Sparse, Sam Ravnborg

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/pragma-once.c                 | 13 ++++++++++++-
 validation/preprocessor/counter2.c       | 16 +++++++++++-----
 validation/preprocessor/counter2.h       |  1 -
 validation/preprocessor/preprocessor20.c | 18 +++++++++++++++---
 validation/preprocessor/preprocessor20.h |  6 ------
 5 files changed, 38 insertions(+), 16 deletions(-)
 delete mode 100644 validation/preprocessor/counter2.h
 delete mode 100644 validation/preprocessor/preprocessor20.h

diff --git a/validation/pragma-once.c b/validation/pragma-once.c
index 5e8b8254..83c0f9f9 100644
--- a/validation/pragma-once.c
+++ b/validation/pragma-once.c
@@ -1,5 +1,16 @@
+/* input-file-1-start */
 #pragma once
-#include "pragma-once.c"
+
+struct s {
+	int i;
+};
+/* input-file-1-end */
+
+/* input-file-2-start */
+#include "$file1"
+#include "$file1"
+/* input-file-2-end */
 /*
  * check-name: #pragma once
+ * check-command: sparse $file2
  */
diff --git a/validation/preprocessor/counter2.c b/validation/preprocessor/counter2.c
index 9883b682..fa15d954 100644
--- a/validation/preprocessor/counter2.c
+++ b/validation/preprocessor/counter2.c
@@ -1,14 +1,20 @@
+/* input-file-1-start */
 __FILE__ __COUNTER__
-#include <counter2.h>
+/* input-file-1-end */
+
+/* input-file-2-start */
 __FILE__ __COUNTER__
+#include "$file1"
+__FILE__ __COUNTER__
+/* input-file-2-end */
 /*
  * check-name: __COUNTER__ #2
- * check-command: sparse -Ipreprocessor -E $file
+ * check-command: sparse -E $file2
  *
  * check-output-start
 
-"preprocessor/counter2.c" 0
-"preprocessor/counter2.h" 1
-"preprocessor/counter2.c" 2
+"$file2" 0
+"$file1" 1
+"$file2" 2
  * check-output-end
  */
diff --git a/validation/preprocessor/counter2.h b/validation/preprocessor/counter2.h
deleted file mode 100644
index 447b70ab..00000000
--- a/validation/preprocessor/counter2.h
+++ /dev/null
@@ -1 +0,0 @@
-__FILE__ __COUNTER__
diff --git a/validation/preprocessor/preprocessor20.c b/validation/preprocessor/preprocessor20.c
index 90e93f37..679a9737 100644
--- a/validation/preprocessor/preprocessor20.c
+++ b/validation/preprocessor/preprocessor20.c
@@ -1,10 +1,22 @@
-#include "preprocessor20.h"
+/* input-file-1-start */
+#ifdef X
+B
+#endif
+#ifndef Y
+A
+#endif
+/* input-file-1-end */
+
+/* input-file-2-start */
+#include "$file1"
 #define X
 #define Y
-#include "preprocessor20.h"
+#include "$file1"
+/* input-file-2-end */
+
 /*
  * check-name: Preprocessor #20
- * check-command: sparse -E $file
+ * check-command: sparse -E $file2
  *
  * check-output-start
 
diff --git a/validation/preprocessor/preprocessor20.h b/validation/preprocessor/preprocessor20.h
deleted file mode 100644
index 322c543a..00000000
--- a/validation/preprocessor/preprocessor20.h
+++ /dev/null
@@ -1,6 +0,0 @@
-#ifdef X
-B
-#endif
-#ifndef Y
-A
-#endif
-- 
2.2.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/3] test-suite: allow filename expansion of the input sections
  2015-02-02  5:17         ` Christopher Li
                             ` (3 preceding siblings ...)
  2015-02-04  2:51           ` [PATCH 3/3] test-suite: consolidate tests that require include files into single test files Luc Van Oostenryck
@ 2015-02-04  3:11           ` Luc Van Oostenryck
  4 siblings, 0 replies; 20+ messages in thread
From: Luc Van Oostenryck @ 2015-02-04  3:11 UTC (permalink / raw)
  To: Christopher Li
  Cc: Josh Triplett, Christian Borntraeger, Jason J. Herne,
	Linux-Sparse, Sam Ravnborg

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Suggested-by: Josh Triplett <josh@joshtriplett.org>
---
 Documentation/test-suite               |  4 ++++
 validation/test-suite                  |  2 ++
 validation/test-suite-file-expansion.c | 22 ++++++++++++++++++++++
 3 files changed, 28 insertions(+)
 create mode 100644 validation/test-suite-file-expansion.c

diff --git a/Documentation/test-suite b/Documentation/test-suite
index e2ce7003..86bee335 100644
--- a/Documentation/test-suite
+++ b/Documentation/test-suite
@@ -35,6 +35,10 @@ check-known-to-fail (optional)
 input-file-1-start / input-file-1-end, / input-file-2-start/ ... (optional)
 	The input files of check-command lies between those two tags.
 	It's only needed when a test requires several files.
+	The '$file1', '$file2', ... strings are special. They will be expanded
+	at run time to the name of each of these files. These strings can be used
+	inside these input sections themselves, the check-command or the
+	check-output section.
 
 
 	Using test-suite
diff --git a/validation/test-suite b/validation/test-suite
index 97d4dd40..8b0cd5e4 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -106,6 +106,7 @@ do_test()
 	input_nr=1
 	while grep -q "input-file-$input_nr-start" "$file"; do
 		sed -n "/input-file-$input_nr-start/,/input-file-$input_nr-end/p" "$file" \
+			| sed "s:\\\$file\\([1-9]\\):$(basename $file).input\\1:" \
 			| grep -v input-file > "$file".input$input_nr
 		eval "file$input_nr=$file.input$input_nr"
 		input_nr=$(($input_nr + 1))
@@ -136,6 +137,7 @@ do_test()
 
 	# grab the expected output
 	sed -n '/check-output-start/,/check-output-end/p' $file \
+		| sed "s:\\\$file\\([1-9]\\):$file.input\\1:" \
 		| grep -v check-output > "$file".output.expected
 	sed -n '/check-error-start/,/check-error-end/p' $file \
 		| grep -v check-error > "$file".error.expected
diff --git a/validation/test-suite-file-expansion.c b/validation/test-suite-file-expansion.c
new file mode 100644
index 00000000..fc4ff8d5
--- /dev/null
+++ b/validation/test-suite-file-expansion.c
@@ -0,0 +1,22 @@
+/* input-file-1-start */
+const char file = __FILE__;
+/* input-file-1-end */
+
+/* input-file-2-start */
+int val = 0;
+#include "$file1"
+const char file = __FILE__;
+/* input-file-2-end */
+
+/*
+ * check-name: test-suite $file* expansion
+ * check-command: sparse -E $file1 $file2
+ *
+ * check-output-start
+
+const char file = "$file1";
+int val = 0;
+const char file = "$file1";
+const char file = "$file2";
+ * check-output-end
+ */
-- 
2.2.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] test-suite: add support for tests case involving several input files
  2015-02-04  2:46           ` [PATCH 1/3] test-suite: add support for tests case involving several input files Luc Van Oostenryck
@ 2015-02-06 15:02             ` Christopher Li
  0 siblings, 0 replies; 20+ messages in thread
From: Christopher Li @ 2015-02-06 15:02 UTC (permalink / raw)
  To: Luc Van Oostenryck
  Cc: Josh Triplett, Christian Borntraeger, Jason J. Herne,
	Linux-Sparse, Sam Ravnborg

On Tue, Feb 3, 2015 at 6:46 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> These input files, instead of being stored in real files and thus polluting/confusing
> the test framework are now placed in a serie of sections inside an unique test file.
> These sections are delimited by new tags: input-file-1-start / input-file-1-end, / input-file-2-start/ ...

Sorry for the late review. Can I summarize the polluting/confusing problem as:
If there is more than one C file was used in one test case, and test suit wll
invoke each of the C file as a separate test case, not  a good thing.

I think merging it into one test file then split them by the test suit
is too complicated.
It is actually easier to maintain them as separate files.

I think the simpler way would be just teach the test suit, some file
was mean to be
part of a separate test case. Just don't invoke it as stand alone test.
We can add some rules like, embed a tag or simply reflect that in the file name.

It will be a smaller change to the test suit as well.

Thanks

Chris

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro
  2015-02-04  2:38           ` Luc Van Oostenryck
@ 2015-02-12 20:16             ` Christopher Li
  0 siblings, 0 replies; 20+ messages in thread
From: Christopher Li @ 2015-02-12 20:16 UTC (permalink / raw)
  To: Luc Van Oostenryck
  Cc: Josh Triplett, Christian Borntraeger, Jason J. Herne,
	Linux-Sparse, Sam Ravnborg

On Tue, Feb 3, 2015 at 6:38 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> Fine for me but Josh had a remark about the third test file (validation/preprocessor/counter3.c)
> abusing a bit the test framework and I shared his feeling.
>
> Here is a patch serie fixing this abuse by extending the test framework.
>

I check in a version with very minor fix for the counter3.c hack.
Counter3.c does a few things unclean, top of which is, it does not invoke
counter3.c at all. It is invoking counter1.c and counter2.c.

I modify coutner3.c to include the couter2.c, that will solve the problem that
counter3.c not using its own source code.

Any way, the change is pushed.

Thanks

Chris

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-02-12 20:16 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-22 20:31 sparse: new feature " multiple initializers" has false positives on MODULE_ALIAS Christian Borntraeger
2015-01-23 16:40 ` Christopher Li
2015-01-23 22:23   ` [PATCH] Teach sparse about the __COUNTER__ predefined macro Luc Van Oostenryck
2015-01-23 22:28     ` Sam Ravnborg
2015-01-23 22:38     ` josh
2015-01-23 23:59       ` Luc Van Oostenryck
2015-01-24  1:29         ` Josh Triplett
2015-01-24 11:27           ` Luc Van Oostenryck
2015-01-24 20:19             ` Josh Triplett
2015-01-24 20:39               ` Luc Van Oostenryck
2015-01-25 20:12         ` Christian Borntraeger
2015-01-28 10:08           ` Christian Borntraeger
2015-02-02  5:17         ` Christopher Li
2015-02-04  2:38           ` Luc Van Oostenryck
2015-02-12 20:16             ` Christopher Li
2015-02-04  2:46           ` [PATCH 1/3] test-suite: add support for tests case involving several input files Luc Van Oostenryck
2015-02-06 15:02             ` Christopher Li
2015-02-04  2:49           ` [PATCH 2/3] test-suite: allow filename expansion of the input sections Luc Van Oostenryck
2015-02-04  2:51           ` [PATCH 3/3] test-suite: consolidate tests that require include files into single test files Luc Van Oostenryck
2015-02-04  3:11           ` [PATCH 2/3] test-suite: allow filename expansion of the input sections Luc Van Oostenryck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).