* [PATCH] sparse/pre-process: introduce "dissect_mode" option to fix dissect/semind
@ 2025-12-17 15:17 Oleg Nesterov
2026-01-16 23:29 ` Chris Li
0 siblings, 1 reply; 7+ messages in thread
From: Oleg Nesterov @ 2025-12-17 15:17 UTC (permalink / raw)
To: Chris Li, Luc Van Oostenryck; +Cc: Alexey Gladkov, linux-sparse
I don't quite understand why does expand() -> collect_arg() path
update ->pos for each token in the input *list, but this breaks
dissect and thus semind.
Test-case:
$ cat -n PP_POS.c
1 #define READ_ONCE(x) x
2 #define WRITE_ONCE(x, y) x = y
3
4 int R, W;
5
6 void func(void)
7 {
8 WRITE_ONCE(
9 W,
10 READ_ONCE(R)
11 );
12 }
$ ./test-dissect PP_POS.c
4:5 def v R int
4:8 def v W int
6:6 def f func void ( ... )
8:3 func -w- v W int
8:3 func -r- v R int
The reported positions of the usage of R and W are wrong,
and thus ./semind doesn't work:
$ ./semind add PP_POS.c
$ ./semind search -l PP_POS.c:10:16
With this patch:
$ ./test-dissect PP_POS.c
4:5 def v R int
4:8 def v W int
6:6 def f func void ( ... )
9:6 func -w- v W int
10:16 func -r- v R int
$ ./semind add PP_POS.c
$ ./semind search -l PP_POS.c:10:16
(def) PP_POS.c 4 5 int R, W;
(-r-) PP_POS.c 10 16 func READ_ONCE(R)
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
dissect.c | 1 +
options.c | 1 +
options.h | 1 +
pre-process.c | 8 +++++---
4 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/dissect.c b/dissect.c
index a6003afa..5fed8e22 100644
--- a/dissect.c
+++ b/dissect.c
@@ -714,6 +714,7 @@ end:
void dissect(struct reporter *rep, struct string_list *filelist)
{
+ dissect_mode = 1;
reporter = rep;
DO_LIST(filelist, file, do_file(file));
diff --git a/options.c b/options.c
index 6ee4d878..0f207e80 100644
--- a/options.c
+++ b/options.c
@@ -71,6 +71,7 @@ int dump_macro_defs = 0;
int dump_macros_only = 0;
int dissect_show_all_symbols = 0;
+int dissect_mode = 0;
unsigned long fdump_ir;
int fhosted = 1;
diff --git a/options.h b/options.h
index c2a9551a..b559254d 100644
--- a/options.h
+++ b/options.h
@@ -71,6 +71,7 @@ extern int dump_macro_defs;
extern int dump_macros_only;
extern int dissect_show_all_symbols;
+extern int dissect_mode;
extern unsigned long fdump_ir;
extern int fhosted;
diff --git a/pre-process.c b/pre-process.c
index 3fb25082..64445881 100644
--- a/pre-process.c
+++ b/pre-process.c
@@ -294,9 +294,11 @@ static struct token *collect_arg(struct token *prev, int vararg, struct position
} else if (match_op(next, ',') && !nesting && !vararg) {
break;
}
- next->pos.stream = pos->stream;
- next->pos.line = pos->line;
- next->pos.pos = pos->pos;
+ if (!dissect_mode) {
+ next->pos.stream = pos->stream;
+ next->pos.line = pos->line;
+ next->pos.pos = pos->pos;
+ }
next->pos.newline = 0;
p = &next->next;
}
--
2.52.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] sparse/pre-process: introduce "dissect_mode" option to fix dissect/semind
2025-12-17 15:17 [PATCH] sparse/pre-process: introduce "dissect_mode" option to fix dissect/semind Oleg Nesterov
@ 2026-01-16 23:29 ` Chris Li
2026-01-17 14:19 ` Oleg Nesterov
0 siblings, 1 reply; 7+ messages in thread
From: Chris Li @ 2026-01-16 23:29 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: Luc Van Oostenryck, Alexey Gladkov, linux-sparse
Hi Oleg,
Slowly catching up my back log from the holidays.
On Wed, Dec 17, 2025 at 7:26 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> I don't quite understand why does expand() -> collect_arg() path
> update ->pos for each token in the input *list, but this breaks
> dissect and thus semind.
That is a good question, I don't understand why it did that either. I
did some digging, inside macro argument list expansion, the "#include
" is not allowed. It is not possible to switch streams here. The
"pos.pos" is for human consumption anyway, it has no effect on the IR
generation. The only visible effect as far as I can tell is related to
the preprocessor "-E" in lib.c:
if (preprocess_only) {
while (!eof_token(token)) {
int prec = 1;
struct token *next = token->next;
const char *separator = "";
if (next->pos.whitespace)
separator = " ";
if (next->pos.newline) {
separator = "\n\t\t\t\t\t";
prec = next->pos.pos; <--- use pos as
indentation level.
if (prec > 4)
prec = 4;
}
printf("%s%.*s", show_token(token), prec, separator);
token = next;
The "-E" output has some indentation enhancement to turn space into
tab level indentation. This "pos" assignment tries to align the
indentation context of the input arguments to the same level of the
expanding macro name.
> Test-case:
>
> $ cat -n PP_POS.c
> 1 #define READ_ONCE(x) x
> 2 #define WRITE_ONCE(x, y) x = y
> 3
> 4 int R, W;
> 5
> 6 void func(void)
> 7 {
> 8 WRITE_ONCE(
> 9 W,
With your patch, when doing "-E", the W will get indentation deeper
than WRITE_ONCE.
> 10 READ_ONCE(R)
> 11 );
> 12 }
>
> $ ./test-dissect PP_POS.c
> 4:5 def v R int
> 4:8 def v W int
> 6:6 def f func void ( ... )
> 8:3 func -w- v W int
> 8:3 func -r- v R int
>
> The reported positions of the usage of R and W are wrong,
> and thus ./semind doesn't work:
It seems to me this enhancement can be used on other macro related
expansions as well.
>
> $ ./semind add PP_POS.c
> $ ./semind search -l PP_POS.c:10:16
>
> With this patch:
>
> $ ./test-dissect PP_POS.c
> 4:5 def v R int
> 4:8 def v W int
> 6:6 def f func void ( ... )
> 9:6 func -w- v W int
> 10:16 func -r- v R int
>
> $ ./semind add PP_POS.c
> $ ./semind search -l PP_POS.c:10:16
> (def) PP_POS.c 4 5 int R, W;
> (-r-) PP_POS.c 10 16 func READ_ONCE(R)
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
> dissect.c | 1 +
> options.c | 1 +
> options.h | 1 +
> pre-process.c | 8 +++++---
> 4 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/dissect.c b/dissect.c
> index a6003afa..5fed8e22 100644
> --- a/dissect.c
> +++ b/dissect.c
> @@ -714,6 +714,7 @@ end:
>
> void dissect(struct reporter *rep, struct string_list *filelist)
> {
> + dissect_mode = 1;
I don't think we need dissect_mode. I am leaning towards enabling it
all the time, maybe except for the preprocessor only mode.
> reporter = rep;
>
> DO_LIST(filelist, file, do_file(file));
> diff --git a/options.c b/options.c
> index 6ee4d878..0f207e80 100644
> --- a/options.c
> +++ b/options.c
> @@ -71,6 +71,7 @@ int dump_macro_defs = 0;
> int dump_macros_only = 0;
>
> int dissect_show_all_symbols = 0;
> +int dissect_mode = 0;
>
> unsigned long fdump_ir;
> int fhosted = 1;
> diff --git a/options.h b/options.h
> index c2a9551a..b559254d 100644
> --- a/options.h
> +++ b/options.h
> @@ -71,6 +71,7 @@ extern int dump_macro_defs;
> extern int dump_macros_only;
>
> extern int dissect_show_all_symbols;
> +extern int dissect_mode;
>
> extern unsigned long fdump_ir;
> extern int fhosted;
> diff --git a/pre-process.c b/pre-process.c
> index 3fb25082..64445881 100644
> --- a/pre-process.c
> +++ b/pre-process.c
> @@ -294,9 +294,11 @@ static struct token *collect_arg(struct token *prev, int vararg, struct position
> } else if (match_op(next, ',') && !nesting && !vararg) {
> break;
> }
> - next->pos.stream = pos->stream;
> - next->pos.line = pos->line;
> - next->pos.pos = pos->pos;
> + if (!dissect_mode) {
> + next->pos.stream = pos->stream;
> + next->pos.line = pos->line;
> + next->pos.pos = pos->pos;
> + }
Maybe change it to "if (preprocess_only)", and fix all the validation
error output of the checker. What do you say?
Overall I feel that without this position overwrite is better to
locate the real location of the argument of the macro. If anyone knows
another reason we should do the position overwrite, please let me
know.
Alternatively we can also fix the preprocessor "-E" indentation
output. Might not be worth the complexity.
Chris
> next->pos.newline = 0;
> p = &next->next;
> }
> --
> 2.52.0
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sparse/pre-process: introduce "dissect_mode" option to fix dissect/semind
2026-01-16 23:29 ` Chris Li
@ 2026-01-17 14:19 ` Oleg Nesterov
2026-01-17 16:32 ` Oleg Nesterov
2026-01-19 0:21 ` Chris Li
0 siblings, 2 replies; 7+ messages in thread
From: Oleg Nesterov @ 2026-01-17 14:19 UTC (permalink / raw)
To: Chris Li; +Cc: Luc Van Oostenryck, Alexey Gladkov, linux-sparse
Hi Chris,
On 01/16, Chris Li wrote:
>
> On Wed, Dec 17, 2025 at 7:26 AM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > I don't quite understand why does expand() -> collect_arg() path
> > update ->pos for each token in the input *list, but this breaks
> > dissect and thus semind.
>
> That is a good question, I don't understand why it did that either. I
> did some digging, inside macro argument list expansion, the "#include
> " is not allowed. It is not possible to switch streams here. The
> "pos.pos" is for human consumption anyway, it has no effect on the IR
> generation. The only visible effect as far as I can tell is related to
> the preprocessor "-E" in lib.c:
>
> if (preprocess_only) {
> while (!eof_token(token)) {
> int prec = 1;
> struct token *next = token->next;
> const char *separator = "";
> if (next->pos.whitespace)
> separator = " ";
> if (next->pos.newline) {
> separator = "\n\t\t\t\t\t";
> prec = next->pos.pos; <--- use pos as
> indentation level.
> if (prec > 4)
> prec = 4;
> }
> printf("%s%.*s", show_token(token), prec, separator);
> token = next;
>
> The "-E" output has some indentation enhancement to turn space into
> tab level indentation. This "pos" assignment tries to align the
> indentation context of the input arguments to the same level of the
> expanding macro name.
Yes, exactly! Initially I tried to simply remove these next->pos.* updates
in collect_arg(), but this causes a lot of failures in validation/preprocessor
(due to extra indentations) and I failed to find a simple fix for the
"if (preprocess_only)" code above. Plus I wasn't comfortable because
I don't understand the intent...
> > void dissect(struct reporter *rep, struct string_list *filelist)
> > {
> > + dissect_mode = 1;
>
> I don't think we need dissect_mode. I am leaning towards enabling it
> all the time, maybe except for the preprocessor only mode.
...
> > + if (!dissect_mode) {
> > + next->pos.stream = pos->stream;
> > + next->pos.line = pos->line;
> > + next->pos.pos = pos->pos;
> > + }
>
> Maybe change it to "if (preprocess_only)", and fix all the validation
> error output of the checker. What do you say?
Agreed! This was my plan B ;)
With this change
- if (!dissect_mode) {
+ if (preprocess_only) {
make check reports 2 failures
-parsing/attr-cleanup.c:10:17: error: argument is not an identifier
+parsing/attr-cleanup.c:10:27: error: argument is not an identifier
-sizeof-void.c:20:14: warning: expression using sizeof(void)
+sizeof-void.c:20:27: warning: expression using sizeof(void)
but the new positions look more correct.
However. I didn't dare to send this patch because other warnings from
sizeof-void.c still blame the column 14, this looks inconsistent...
But perhaps we don't really care?
So. I am going to update the changelog and send the trivial V2 below.
Will you agree?
Oleg.
---
diff --git a/pre-process.c b/pre-process.c
index 3fb25082..a4bb6cb6 100644
--- a/pre-process.c
+++ b/pre-process.c
@@ -294,9 +294,11 @@ static struct token *collect_arg(struct token *prev, int vararg, struct position
} else if (match_op(next, ',') && !nesting && !vararg) {
break;
}
- next->pos.stream = pos->stream;
- next->pos.line = pos->line;
- next->pos.pos = pos->pos;
+ if (preprocess_only) {
+ next->pos.stream = pos->stream;
+ next->pos.line = pos->line;
+ next->pos.pos = pos->pos;
+ }
next->pos.newline = 0;
p = &next->next;
}
diff --git a/validation/parsing/attr-cleanup.c b/validation/parsing/attr-cleanup.c
index ac64649c..fa3cb1ca 100644
--- a/validation/parsing/attr-cleanup.c
+++ b/validation/parsing/attr-cleanup.c
@@ -24,7 +24,7 @@ int test(int n)
* check-command: sparse -Wunknown-attribute $file
*
* check-error-start
-parsing/attr-cleanup.c:10:17: error: argument is not an identifier
+parsing/attr-cleanup.c:10:27: error: argument is not an identifier
parsing/attr-cleanup.c:11:39: error: an argument is expected for attribute 'cleanup'
parsing/attr-cleanup.c:12:40: error: an argument is expected for attribute 'cleanup'
parsing/attr-cleanup.c:13:43: error: Expected ) after attribute's argument'
diff --git a/validation/sizeof-void.c b/validation/sizeof-void.c
index 0fd917a2..6792ff02 100644
--- a/validation/sizeof-void.c
+++ b/validation/sizeof-void.c
@@ -36,7 +36,7 @@ sizeof-void.c:16:14: warning: expression using sizeof(void)
sizeof-void.c:17:14: warning: expression using sizeof(void)
sizeof-void.c:18:14: warning: expression using sizeof(void)
sizeof-void.c:19:14: warning: expression using sizeof(void)
-sizeof-void.c:20:14: warning: expression using sizeof(void)
+sizeof-void.c:20:27: warning: expression using sizeof(void)
sizeof-void.c:21:14: warning: expression using sizeof(void)
sizeof-void.c:22:14: warning: expression using sizeof(void)
sizeof-void.c:23:14: warning: expression using sizeof(void)
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] sparse/pre-process: introduce "dissect_mode" option to fix dissect/semind
2026-01-17 14:19 ` Oleg Nesterov
@ 2026-01-17 16:32 ` Oleg Nesterov
2026-01-19 0:23 ` Chris Li
2026-01-19 0:21 ` Chris Li
1 sibling, 1 reply; 7+ messages in thread
From: Oleg Nesterov @ 2026-01-17 16:32 UTC (permalink / raw)
To: Chris Li; +Cc: Luc Van Oostenryck, Alexey Gladkov, linux-sparse
On 01/17, Oleg Nesterov wrote:
>
> Agreed! This was my plan B ;)
>
> With this change
>
> - if (!dissect_mode) {
> + if (preprocess_only) {
>
> make check reports 2 failures
>
> -parsing/attr-cleanup.c:10:17: error: argument is not an identifier
> +parsing/attr-cleanup.c:10:27: error: argument is not an identifier
>
> -sizeof-void.c:20:14: warning: expression using sizeof(void)
> +sizeof-void.c:20:27: warning: expression using sizeof(void)
>
> but the new positions look more correct.
>
> However. I didn't dare to send this patch because other warnings from
> sizeof-void.c still blame the column 14, this looks inconsistent...
> But perhaps we don't really care?
On a 2nd thought...
Unlike other warnings, this one (sizeof-void.c:20:27) refers to the
inner "sizeof *ptr", so I think that this patch fixes the reported
position. So yes, I think we don't care even if the new column == 27
differs from other warnings.
What do you think?
Oleg.
> So. I am going to update the changelog and send the trivial V2 below.
>
> Will you agree?
>
> Oleg.
> ---
>
> diff --git a/pre-process.c b/pre-process.c
> index 3fb25082..a4bb6cb6 100644
> --- a/pre-process.c
> +++ b/pre-process.c
> @@ -294,9 +294,11 @@ static struct token *collect_arg(struct token *prev, int vararg, struct position
> } else if (match_op(next, ',') && !nesting && !vararg) {
> break;
> }
> - next->pos.stream = pos->stream;
> - next->pos.line = pos->line;
> - next->pos.pos = pos->pos;
> + if (preprocess_only) {
> + next->pos.stream = pos->stream;
> + next->pos.line = pos->line;
> + next->pos.pos = pos->pos;
> + }
> next->pos.newline = 0;
> p = &next->next;
> }
> diff --git a/validation/parsing/attr-cleanup.c b/validation/parsing/attr-cleanup.c
> index ac64649c..fa3cb1ca 100644
> --- a/validation/parsing/attr-cleanup.c
> +++ b/validation/parsing/attr-cleanup.c
> @@ -24,7 +24,7 @@ int test(int n)
> * check-command: sparse -Wunknown-attribute $file
> *
> * check-error-start
> -parsing/attr-cleanup.c:10:17: error: argument is not an identifier
> +parsing/attr-cleanup.c:10:27: error: argument is not an identifier
> parsing/attr-cleanup.c:11:39: error: an argument is expected for attribute 'cleanup'
> parsing/attr-cleanup.c:12:40: error: an argument is expected for attribute 'cleanup'
> parsing/attr-cleanup.c:13:43: error: Expected ) after attribute's argument'
> diff --git a/validation/sizeof-void.c b/validation/sizeof-void.c
> index 0fd917a2..6792ff02 100644
> --- a/validation/sizeof-void.c
> +++ b/validation/sizeof-void.c
> @@ -36,7 +36,7 @@ sizeof-void.c:16:14: warning: expression using sizeof(void)
> sizeof-void.c:17:14: warning: expression using sizeof(void)
> sizeof-void.c:18:14: warning: expression using sizeof(void)
> sizeof-void.c:19:14: warning: expression using sizeof(void)
> -sizeof-void.c:20:14: warning: expression using sizeof(void)
> +sizeof-void.c:20:27: warning: expression using sizeof(void)
> sizeof-void.c:21:14: warning: expression using sizeof(void)
> sizeof-void.c:22:14: warning: expression using sizeof(void)
> sizeof-void.c:23:14: warning: expression using sizeof(void)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sparse/pre-process: introduce "dissect_mode" option to fix dissect/semind
2026-01-17 14:19 ` Oleg Nesterov
2026-01-17 16:32 ` Oleg Nesterov
@ 2026-01-19 0:21 ` Chris Li
1 sibling, 0 replies; 7+ messages in thread
From: Chris Li @ 2026-01-19 0:21 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: Luc Van Oostenryck, Alexey Gladkov, linux-sparse
On Sat, Jan 17, 2026 at 6:19 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> Hi Chris,
>
> On 01/16, Chris Li wrote:
> >
> > On Wed, Dec 17, 2025 at 7:26 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > >
> > > I don't quite understand why does expand() -> collect_arg() path
> > > update ->pos for each token in the input *list, but this breaks
> > > dissect and thus semind.
> >
> > That is a good question, I don't understand why it did that either. I
> > did some digging, inside macro argument list expansion, the "#include
> > " is not allowed. It is not possible to switch streams here. The
> > "pos.pos" is for human consumption anyway, it has no effect on the IR
> > generation. The only visible effect as far as I can tell is related to
> > the preprocessor "-E" in lib.c:
> >
> > if (preprocess_only) {
> > while (!eof_token(token)) {
> > int prec = 1;
> > struct token *next = token->next;
> > const char *separator = "";
> > if (next->pos.whitespace)
> > separator = " ";
> > if (next->pos.newline) {
> > separator = "\n\t\t\t\t\t";
> > prec = next->pos.pos; <--- use pos as
> > indentation level.
> > if (prec > 4)
> > prec = 4;
> > }
> > printf("%s%.*s", show_token(token), prec, separator);
> > token = next;
> >
> > The "-E" output has some indentation enhancement to turn space into
> > tab level indentation. This "pos" assignment tries to align the
> > indentation context of the input arguments to the same level of the
> > expanding macro name.
>
> Yes, exactly! Initially I tried to simply remove these next->pos.* updates
> in collect_arg(), but this causes a lot of failures in validation/preprocessor
> (due to extra indentations) and I failed to find a simple fix for the
> "if (preprocess_only)" code above. Plus I wasn't comfortable because
> I don't understand the intent...
As far as I can tell, the new position report is more useful to the
reader. I haven't heard any objections yet. Let's put it on the
sparse-dev for a bit then merge into sparse. In the worst case there
is some burning reason to use the old behavior, we can always change
the behavior back. It is only software and it is easy to fix.
>
> > > void dissect(struct reporter *rep, struct string_list *filelist)
> > > {
> > > + dissect_mode = 1;
> >
> > I don't think we need dissect_mode. I am leaning towards enabling it
> > all the time, maybe except for the preprocessor only mode.
>
> ...
>
> > > + if (!dissect_mode) {
> > > + next->pos.stream = pos->stream;
> > > + next->pos.line = pos->line;
> > > + next->pos.pos = pos->pos;
> > > + }
> >
> > Maybe change it to "if (preprocess_only)", and fix all the validation
> > error output of the checker. What do you say?
>
> Agreed! This was my plan B ;)
>
> With this change
>
> - if (!dissect_mode) {
> + if (preprocess_only) {
>
> make check reports 2 failures
>
> -parsing/attr-cleanup.c:10:17: error: argument is not an identifier
> +parsing/attr-cleanup.c:10:27: error: argument is not an identifier
>
> -sizeof-void.c:20:14: warning: expression using sizeof(void)
> +sizeof-void.c:20:27: warning: expression using sizeof(void)
>
> but the new positions look more correct.
>
> However. I didn't dare to send this patch because other warnings from
> sizeof-void.c still blame the column 14, this looks inconsistent...
> But perhaps we don't really care?
Well, we care in the sense that we don't want unnecessary check
failures. But we can update the expected output of the validation
check to silence the error.
>
> So. I am going to update the changelog and send the trivial V2 below.
>
> Will you agree?
Agree. As far as I can tell. The new position is more desirable. Let's
switch to the new position.
Chris
>
> Oleg.
> ---
>
> diff --git a/pre-process.c b/pre-process.c
> index 3fb25082..a4bb6cb6 100644
> --- a/pre-process.c
> +++ b/pre-process.c
> @@ -294,9 +294,11 @@ static struct token *collect_arg(struct token *prev, int vararg, struct position
> } else if (match_op(next, ',') && !nesting && !vararg) {
> break;
> }
> - next->pos.stream = pos->stream;
> - next->pos.line = pos->line;
> - next->pos.pos = pos->pos;
> + if (preprocess_only) {
> + next->pos.stream = pos->stream;
> + next->pos.line = pos->line;
> + next->pos.pos = pos->pos;
> + }
> next->pos.newline = 0;
> p = &next->next;
> }
> diff --git a/validation/parsing/attr-cleanup.c b/validation/parsing/attr-cleanup.c
> index ac64649c..fa3cb1ca 100644
> --- a/validation/parsing/attr-cleanup.c
> +++ b/validation/parsing/attr-cleanup.c
> @@ -24,7 +24,7 @@ int test(int n)
> * check-command: sparse -Wunknown-attribute $file
> *
> * check-error-start
> -parsing/attr-cleanup.c:10:17: error: argument is not an identifier
> +parsing/attr-cleanup.c:10:27: error: argument is not an identifier
> parsing/attr-cleanup.c:11:39: error: an argument is expected for attribute 'cleanup'
> parsing/attr-cleanup.c:12:40: error: an argument is expected for attribute 'cleanup'
> parsing/attr-cleanup.c:13:43: error: Expected ) after attribute's argument'
> diff --git a/validation/sizeof-void.c b/validation/sizeof-void.c
> index 0fd917a2..6792ff02 100644
> --- a/validation/sizeof-void.c
> +++ b/validation/sizeof-void.c
> @@ -36,7 +36,7 @@ sizeof-void.c:16:14: warning: expression using sizeof(void)
> sizeof-void.c:17:14: warning: expression using sizeof(void)
> sizeof-void.c:18:14: warning: expression using sizeof(void)
> sizeof-void.c:19:14: warning: expression using sizeof(void)
> -sizeof-void.c:20:14: warning: expression using sizeof(void)
> +sizeof-void.c:20:27: warning: expression using sizeof(void)
> sizeof-void.c:21:14: warning: expression using sizeof(void)
> sizeof-void.c:22:14: warning: expression using sizeof(void)
> sizeof-void.c:23:14: warning: expression using sizeof(void)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sparse/pre-process: introduce "dissect_mode" option to fix dissect/semind
2026-01-17 16:32 ` Oleg Nesterov
@ 2026-01-19 0:23 ` Chris Li
2026-01-19 12:32 ` Oleg Nesterov
0 siblings, 1 reply; 7+ messages in thread
From: Chris Li @ 2026-01-19 0:23 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: Luc Van Oostenryck, Alexey Gladkov, linux-sparse
On Sat, Jan 17, 2026 at 8:32 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 01/17, Oleg Nesterov wrote:
> >
> > Agreed! This was my plan B ;)
> >
> > With this change
> >
> > - if (!dissect_mode) {
> > + if (preprocess_only) {
> >
> > make check reports 2 failures
> >
> > -parsing/attr-cleanup.c:10:17: error: argument is not an identifier
> > +parsing/attr-cleanup.c:10:27: error: argument is not an identifier
> >
> > -sizeof-void.c:20:14: warning: expression using sizeof(void)
> > +sizeof-void.c:20:27: warning: expression using sizeof(void)
> >
> > but the new positions look more correct.
> >
> > However. I didn't dare to send this patch because other warnings from
> > sizeof-void.c still blame the column 14, this looks inconsistent...
> > But perhaps we don't really care?
>
> On a 2nd thought...
>
> Unlike other warnings, this one (sizeof-void.c:20:27) refers to the
> inner "sizeof *ptr", so I think that this patch fixes the reported
> position. So yes, I think we don't care even if the new column == 27
> differs from other warnings.
>
> What do you think?
I would just update the checker to have the new expected value
matching what new pos so validation can pass without errors.
Chris
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sparse/pre-process: introduce "dissect_mode" option to fix dissect/semind
2026-01-19 0:23 ` Chris Li
@ 2026-01-19 12:32 ` Oleg Nesterov
0 siblings, 0 replies; 7+ messages in thread
From: Oleg Nesterov @ 2026-01-19 12:32 UTC (permalink / raw)
To: Chris Li; +Cc: Luc Van Oostenryck, Alexey Gladkov, linux-sparse
On 01/18, Chris Li wrote:
>
> On Sat, Jan 17, 2026 at 8:32 AM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > On a 2nd thought...
> >
> > Unlike other warnings, this one (sizeof-void.c:20:27) refers to the
> > inner "sizeof *ptr", so I think that this patch fixes the reported
> > position. So yes, I think we don't care even if the new column == 27
> > differs from other warnings.
> >
> > What do you think?
>
> I would just update the checker to have the new expected value
> matching what new pos so validation can pass without errors.
Yes, this is what I did. See
[PATCH] sparse/pre-process: don't update next->pos in collect_arg()
https://lore.kernel.org/all/aWz0V_zQ47afKFJy@redhat.com/
I was confused and tried to confuse you... let me explain. With this patch
./sparse -Wpointer-arith validation/sizeof-void.c outputs
validation/sizeof-void.c:16:14: warning: expression using sizeof(void)
validation/sizeof-void.c:17:14: warning: expression using sizeof(void)
validation/sizeof-void.c:18:14: warning: expression using sizeof(void)
validation/sizeof-void.c:19:14: warning: expression using sizeof(void)
validation/sizeof-void.c:20:27: warning: expression using sizeof(void) // changed
validation/sizeof-void.c:21:14: warning: expression using sizeof(void)
validation/sizeof-void.c:22:14: warning: expression using sizeof(void)
validation/sizeof-void.c:23:14: warning: expression using sizeof(void)
and somehow I wrongly came to conlusion that my patch is incomplete or
inconsistent because it only corrects the warning's position for the
line 20.
Now that I actually looked at validation/sizeof-void.c, I see that the
code at line 20
s += is_constexpr(sizeof *ptr);
differs in that it is the inner "sizeof *ptr" which triggers the warning,
so I think the patch is fine.
Thank you,
Oleg.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-01-19 12:33 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-17 15:17 [PATCH] sparse/pre-process: introduce "dissect_mode" option to fix dissect/semind Oleg Nesterov
2026-01-16 23:29 ` Chris Li
2026-01-17 14:19 ` Oleg Nesterov
2026-01-17 16:32 ` Oleg Nesterov
2026-01-19 0:23 ` Chris Li
2026-01-19 12:32 ` Oleg Nesterov
2026-01-19 0:21 ` Chris Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox