All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.