* 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
* 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
* [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
* 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
* [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
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).