* 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 --]
lib.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
pre-process.c | 12 ++++-----
symbol.c | 2 +-
token.h | 7 ++---
tokenize.c | 14 +++++++---
5 files changed, 103 insertions(+), 18 deletions(-)
diff --git a/lib.c b/lib.c
index 4e8d7b45..dcbbb5b3 100644
--- a/lib.c
+++ b/lib.c
@@ -49,22 +49,98 @@
#include "version.h"
#include "bits.h"
+static int prettify(const char **fnamep)
+{
+ const char *name = *fnamep;
+ int len = strlen(name);
+
+ if (len > 2 && !memcmp(name, "./", 2)) {
+ name += 2;
+ len -= 2;
+ }
+
+ *fnamep = name;
+ return len;
+}
+
+static const char *show_include_chain(int stream, const char *base)
+{
+ static char buffer[200];
+ int len = 0;
+
+ while ((stream = stream_prev(stream)) >= 0) {
+ const char *p = stream_name(stream);
+ int pretty_len;
+
+ if (p == base)
+ break;
+
+ pretty_len = prettify(&p);
+ if (pretty_len <= 0)
+ break;
+
+ /*
+ * At worst, we'll need " (through %s, ...)" in addition to the
+ * new filename
+ */
+ if (pretty_len + len + 20 > sizeof(buffer)) {
+ if (!len)
+ return "";
+ memcpy(buffer+len, ", ...", 5);
+ len += 5;
+ break;
+ }
+
+ if (!len) {
+ memcpy(buffer, " (through ", 10);
+ len = 10;
+ } else {
+ buffer[len++] = ',';
+ buffer[len++] = ' ';
+ }
+
+ memcpy(buffer+len, p, pretty_len);
+ len += pretty_len;
+ }
+ if (!len)
+ return "";
+
+ buffer[len] = ')';
+ buffer[len+1] = 0;
+ return buffer;
+}
+
+static const char *show_stream_name(struct position pos)
+{
+ const char *name = stream_name(pos.stream);
+ static const char *last;
+
+ if (name == base_filename)
+ return name;
+ if (name == last)
+ return name;
+ last = name;
+
+ fprintf(stderr, "%s: note: in included file%s:\n",
+ base_filename,
+ show_include_chain(pos.stream, base_filename));
+ return name;
+}
static void do_warn(const char *type, struct position pos, const char * fmt, va_list args)
{
static char buffer[512];
- const char *name;
/* Shut up warnings if position is bad_token.pos */
if (pos.type == TOKEN_BAD)
return;
vsprintf(buffer, fmt, args);
- name = stream_name(pos.stream);
-
+
fflush(stdout);
fprintf(stderr, "%s:%d:%d: %s%s%s\n",
- name, pos.line, pos.pos, diag_prefix, type, buffer);
+ show_stream_name(pos), pos.line, pos.pos,
+ diag_prefix, type, buffer);
}
static int show_info = 1;
@@ -275,7 +351,7 @@ static struct symbol_list *sparse_file(const char *filename)
base_filename = filename;
// Tokenize the input stream
- token = tokenize(filename, fd, NULL, includepath);
+ token = tokenize(filename, fd, -1, NULL, includepath);
close(fd);
return sparse_tokenstream(token);
diff --git a/pre-process.c b/pre-process.c
index 403e3507..ab974b22 100644
--- a/pre-process.c
+++ b/pre-process.c
@@ -894,7 +894,7 @@ static void set_stream_include_path(struct stream *stream)
#define PATH_MAX 4096 // for Hurd where it's not defined
#endif
-static int try_include(const char *path, const char *filename, int flen, struct token **where, const char **next_path)
+static int try_include(struct position pos, const char *path, const char *filename, int flen, struct token **where, const char **next_path)
{
int fd;
int plen = strlen(path);
@@ -910,8 +910,8 @@ static int try_include(const char *path, const char *filename, int flen, struct
return 1;
fd = open(fullname, O_RDONLY);
if (fd >= 0) {
- char *streamname = xmemdup(fullname, plen + flen);
- *where = tokenize(streamname, fd, *where, next_path);
+ char *streamname = streamname = xmemdup(fullname, plen + flen);
+ *where = tokenize(streamname, fd, pos.stream, *where, next_path);
close(fd);
return 1;
}
@@ -923,7 +923,7 @@ static int do_include_path(const char **pptr, struct token **list, struct token
const char *path;
while ((path = *pptr++) != NULL) {
- if (!try_include(path, filename, flen, list, pptr))
+ if (!try_include(token->pos, path, filename, flen, list, pptr))
continue;
return 1;
}
@@ -966,7 +966,7 @@ static int handle_include_path(struct stream *stream, struct token **list, struc
/* Absolute path? */
if (filename[0] == '/') {
- if (try_include("", filename, flen, list, includepath))
+ if (try_include(token->pos, "", filename, flen, list, includepath))
return 0;
goto out;
}
@@ -2091,7 +2091,7 @@ static void create_arglist(struct symbol *sym, int count)
static void init_preprocessor(void)
{
int i;
- int stream = init_stream("preprocessor", -1, includepath);
+ int stream = init_stream("preprocessor", -1, includepath, -1);
static struct {
const char *name;
int (*handler)(struct stream *, struct token **, struct token *);
diff --git a/symbol.c b/symbol.c
index c0ca79e4..2cc9f82d 100644
--- a/symbol.c
+++ b/symbol.c
@@ -776,7 +776,7 @@ struct symbol zero_int;
void init_symbols(void)
{
- int stream = init_stream("builtin", -1, includepath);
+ int stream = init_stream("builtin", -1, includepath, -1);
#define __IDENT(n,str,res) \
hash_ident(&n)
diff --git a/token.h b/token.h
index c5fdf3d0..6d2b0b65 100644
--- a/token.h
+++ b/token.h
@@ -49,7 +49,7 @@ enum constantfile {
extern const char *includepath[];
struct stream {
- int fd;
+ int fd, prev;
const char *name;
const char *path; // input-file path - see set_stream_include_path()
const char **next_path;
@@ -214,7 +214,8 @@ static inline struct token *containing_token(struct token **p)
extern struct token eof_token_entry;
#define eof_token(x) ((x) == &eof_token_entry)
-extern int init_stream(const char *, int fd, const char **next_path);
+extern int init_stream(const char *, int fd, const char **next_path, int prev_stream);
+extern int stream_prev(int stream);
extern const char *stream_name(int stream);
extern struct ident *hash_ident(struct ident *);
extern struct ident *built_in_ident(const char *);
@@ -224,7 +225,7 @@ extern const char *show_ident(const struct ident *);
extern const char *show_string(const struct string *string);
extern const char *show_token(const struct token *);
extern const char *quote_token(const struct token *);
-extern struct token * tokenize(const char *, int, struct token *, const char **next_path);
+extern struct token * tokenize(const char *, int, int, struct token *, const char **next_path);
extern struct token * tokenize_buffer(void *, unsigned long, struct token **);
extern void show_identifier_stats(void);
diff --git a/tokenize.c b/tokenize.c
index d3371e1e..c5ba6e6b 100644
--- a/tokenize.c
+++ b/tokenize.c
@@ -62,6 +62,13 @@ const char *stream_name(int stream)
return input_streams[stream].name;
}
+int stream_prev(int stream)
+{
+ if (stream < 0 || stream > input_stream_nr)
+ return -1;
+ return input_streams[stream].prev;
+}
+
static struct position stream_pos(stream_t *stream)
{
struct position pos;
@@ -300,7 +307,7 @@ int *hash_stream(const char *name)
return input_stream_hashes + hash;
}
-int init_stream(const char *name, int fd, const char **next_path)
+int init_stream(const char *name, int fd, const char **next_path, int prev_stream)
{
int stream = input_stream_nr, *hash;
struct stream *current;
@@ -319,6 +326,7 @@ int init_stream(const char *name, int fd, const char **next_path)
current->next_path = next_path;
current->path = NULL;
current->constant = CONSTANT_FILE_MAYBE;
+ current->prev = prev_stream;
input_stream_nr = stream+1;
hash = hash_stream(name);
current->next_stream = *hash;
@@ -1006,14 +1014,14 @@ struct token * tokenize_buffer(void *buffer, unsigned long size, struct token **
return begin;
}
-struct token * tokenize(const char *name, int fd, struct token *endtoken, const char **next_path)
+struct token * tokenize(const char *name, int fd, int prev_stream, struct token *endtoken, const char **next_path)
{
struct token *begin, *end;
stream_t stream;
unsigned char buffer[BUFSIZE];
int idx;
- idx = init_stream(name, fd, next_path);
+ idx = init_stream(name, fd, next_path, prev_stream);
if (idx < 0) {
// info(endtoken->pos, "File %s is const", name);
return endtoken;
^ permalink raw reply related [flat|nested] 3+ messages in thread