Linux-Sparse Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] warnings: ensure the source filename is shown at least once
@ 2020-07-18 18:39 Luc Van Oostenryck
  2020-07-18 20:21 ` Linus Torvalds
  0 siblings, 1 reply; 3+ messages in thread
From: Luc Van Oostenryck @ 2020-07-18 18:39 UTC (permalink / raw)
  To: linux-sparse; +Cc: Luc Van Oostenryck

When checking a series of files, if some warnings or errors
are issued but only coming from some includes, it's not possible
to identify which source file is responsible since its filename
is not displayed.

So, if the first warning is from a file other than the source
file, display first a note coming from the source file itself.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 lib.c                         | 13 +++++++++++++
 validation/cast-kinds-check.c |  1 +
 2 files changed, 14 insertions(+)

diff --git a/lib.c b/lib.c
index 4e8d7b451747..b94c2c83c1c5 100644
--- a/lib.c
+++ b/lib.c
@@ -50,6 +50,18 @@
 #include "bits.h"
 
 
+static void show_top_filename(const char *name)
+{
+	static const char *last;
+
+	if (name == base_filename)
+		return;
+	if (name == last)
+		return;
+	fprintf(stderr, "%s: note: in included file:\n", base_filename);
+	last = name;
+}
+
 static void do_warn(const char *type, struct position pos, const char * fmt, va_list args)
 {
 	static char buffer[512];
@@ -63,6 +75,7 @@ static void do_warn(const char *type, struct position pos, const char * fmt, va_
 	name = stream_name(pos.stream);
 		
 	fflush(stdout);
+	show_top_filename(name);
 	fprintf(stderr, "%s:%d:%d: %s%s%s\n",
 		name, pos.line, pos.pos, diag_prefix, type, buffer);
 }
diff --git a/validation/cast-kinds-check.c b/validation/cast-kinds-check.c
index 0c0cd67368a3..32a106d413da 100644
--- a/validation/cast-kinds-check.c
+++ b/validation/cast-kinds-check.c
@@ -6,6 +6,7 @@
  * check-assert: sizeof(long) == 8
  *
  * check-error-start
+cast-kinds-check.c: note: in included file:
 optim/cast-kinds.c:5:45: warning: cast drops bits
 optim/cast-kinds.c:6:47: warning: cast drops bits
 optim/cast-kinds.c:7:46: warning: cast drops bits
-- 
2.27.0


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

* Re: [PATCH] warnings: ensure the source filename is shown at least once
  2020-07-18 18:39 [PATCH] warnings: ensure the source filename is shown at least once Luc Van Oostenryck
@ 2020-07-18 20:21 ` Linus Torvalds
  2020-07-18 23:36   ` Luc Van Oostenryck
  0 siblings, 1 reply; 3+ messages in thread
From: Linus Torvalds @ 2020-07-18 20:21 UTC (permalink / raw)
  To: Luc Van Oostenryck; +Cc: Sparse Mailing-list


[-- Attachment #1: Type: text/plain, Size: 1778 bytes --]

On Sat, Jul 18, 2020 at 11:40 AM Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> When checking a series of files, if some warnings or errors
> are issued but only coming from some includes, it's not possible
> to identify which source file is responsible since its filename
> is not displayed.
>
> So, if the first warning is from a file other than the source
> file, display first a note coming from the source file itself.

This really isn't enough when the include chain is deeper and more complex.

How about something a bit more complex? This is only lightly tested,
but I don't have time for anything more right now..

It results in things like

    kernel/exit.c: note: in included file (through
include/linux/sched/signal.h, include/linux/rcuwait.h,
include/linux/percpu-rwsem.h, include/linux/fs.h,
include/linux/huge_mm.h, include/linux/mm.h):
    ./include/linux/sched/task.h:115:34: warning: context imbalance in
'wait_task_zombie' - unexpected unlock
    ./include/linux/sched/task.h:115:34: warning: context imbalance in
'wait_task_stopped' - unexpected unlock
    ./include/linux/sched/task.h:115:34: warning: context imbalance in
'wait_task_continued' - unexpected unlock

to give an example of a warning happening in a header file that got
included through a deeper chain.

That stream chaining information might perhaps be useful for other cases too?

Would it be better to save the whole 'pos' for the chain, so that
you'd get line numbers etc for the chain? Probably. As mentioned, this
is a quick "how about something like this".

If you extend it and do more testing, you can have my sign-off, or
just take ownership of the patch entirely with my ack..

Now off for more kernel stuff after a quick sparse excursion...

                   Linus

[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 8070 bytes --]

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

* Re: [PATCH] warnings: ensure the source filename is shown at least once
  2020-07-18 20:21 ` Linus Torvalds
@ 2020-07-18 23:36   ` Luc Van Oostenryck
  0 siblings, 0 replies; 3+ messages in thread
From: Luc Van Oostenryck @ 2020-07-18 23:36 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Sparse Mailing-list

On Sat, Jul 18, 2020 at 01:21:35PM -0700, Linus Torvalds wrote:
> On Sat, Jul 18, 2020 at 11:40 AM Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> >
> > When checking a series of files, if some warnings or errors
> > are issued but only coming from some includes, it's not possible
> > to identify which source file is responsible since its filename
> > is not displayed.
> >
> > So, if the first warning is from a file other than the source
> > file, display first a note coming from the source file itself.
> 
> This really isn't enough when the include chain is deeper and more complex.

Yes, my use case was much more limited.
 
> How about something a bit more complex? This is only lightly tested,
> but I don't have time for anything more right now..
> 
> It results in things like

I like it a lot and it works nicely.
 
> That stream chaining information might perhaps be useful for other cases too?

For sparse itself, I don't know, but for some other tools that would
analyze the code/#include structure, yes surely.
 
> Would it be better to save the whole 'pos' for the chain, so that
> you'd get line numbers etc for the chain? Probably. As mentioned, this
> is a quick "how about something like this".

I added this in a following patch.
 
> If you extend it and do more testing, you can have my sign-off, or
> just take ownership of the patch entirely with my ack..

I only removed a repetition in an initialization, so I added your s-o-b.

> Now off for more kernel stuff after a quick sparse excursion...

Yes, sure!
Thanks a lot because it's, I think, a really nice improvement.
-- Luc

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-18 18:39 [PATCH] warnings: ensure the source filename is shown at least once Luc Van Oostenryck
2020-07-18 20:21 ` Linus Torvalds
2020-07-18 23:36   ` Luc Van Oostenryck

Linux-Sparse Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-sparse/0 linux-sparse/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-sparse linux-sparse/ https://lore.kernel.org/linux-sparse \
		linux-sparse@vger.kernel.org
	public-inbox-index linux-sparse

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-sparse


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git