linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] Handling multiple -include directives
@ 2006-12-22  7:06 Pavel Roskin
  2006-12-22  7:36 ` Pavel Roskin
  0 siblings, 1 reply; 4+ messages in thread
From: Pavel Roskin @ 2006-12-22  7:06 UTC (permalink / raw)
  To: linux-sparse

Hello!

sparse doesn't handle multiple -include directives.  Variables "include"
and "include_fd" have file scope in lib.c.  The file for inclusion is
processed only once in sparse_initial() after the command line has been
processed.

It seems to me that the existing add_pre_buffer() mechanism can be used
instead.  I'm just a bit worried why it wasn't done like this in the
first place.

---
Handle multiple -include directives

Signed-off-by: Pavel Roskin <proski@gnu.org>
---

 lib.c |   26 ++++++--------------------
 1 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/lib.c b/lib.c
index 51c415d..3ea3bde 100644
--- a/lib.c
+++ b/lib.c
@@ -191,8 +191,6 @@ int Wenum_mismatch = 1;
 int Wdo_while = 1;
 int Wuninitialized = 1;
 int preprocess_only;
-char *include;
-int include_fd = -1;
 
 
 void add_pre_buffer(const char *fmt, ...)
@@ -266,21 +264,15 @@ static char **handle_switch_i(char *arg, char **next)
 {
 	if (*next && !strcmp(arg, "include")) {
 		char *name = *++next;
-		int fd = open(name, O_RDONLY);
-
-		include_fd = fd;
-		include = name;
-		if (fd < 0)
-			perror(name);
+		if (!name)
+			die("missing argument for -include option");
+		add_pre_buffer("#include \"%s\"\n", name);
 	}
 	if (*next && !strcmp(arg, "imacros")) {
 		char *name = *++next;
-		int fd = open(name, O_RDONLY);
-
-		include_fd = fd;
-		include = name;
-		if (fd < 0)
-			perror(name);
+		if (!name)
+			die("missing argument for -include option");
+		add_pre_buffer("#include \"%s\"\n", name);
 	}
 	else if (*next && !strcmp(arg, "isystem")) {
 		char *path = *++next;
@@ -624,12 +616,6 @@ static struct symbol_list *sparse_initial(void)
 {
 	struct token *token;
 
-	// Prepend any "include" file to the stream.
-	// We're in global scope, it will affect all files!
-	token = NULL;
-	if (include_fd >= 0)
-		token = tokenize(include, include_fd, NULL, includepath);
-
 	// Prepend the initial built-in stream
 	token = tokenize_buffer(pre_buffer, pre_buffer_size, token);
 	return sparse_tokenstream(token);


-- 
Regards,
Pavel Roskin

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

* Re: [RFC PATCH] Handling multiple -include directives
  2006-12-22  7:06 [RFC PATCH] Handling multiple -include directives Pavel Roskin
@ 2006-12-22  7:36 ` Pavel Roskin
  2006-12-22  9:47   ` Christopher Li
  0 siblings, 1 reply; 4+ messages in thread
From: Pavel Roskin @ 2006-12-22  7:36 UTC (permalink / raw)
  To: linux-sparse

On Fri, 2006-12-22 at 02:06 -0500, Pavel Roskin wrote:
> It seems to me that the existing add_pre_buffer() mechanism can be used
> instead.  I'm just a bit worried why it wasn't done like this in the
> first place.

There was a reason to worry.  Now create_builtin_stream() is run after
the includes have been processed, so that e.g. the Linux compiler.h
tells me that my compiler is too old (because it was included from the
command line before __GNUC__ was defined).

The initialization order needs some untangling.  It's too intertwined
with the command line processing.

-- 
Regards,
Pavel Roskin

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

* Re: [RFC PATCH] Handling multiple -include directives
  2006-12-22  7:36 ` Pavel Roskin
@ 2006-12-22  9:47   ` Christopher Li
  2006-12-22 22:27     ` Pavel Roskin
  0 siblings, 1 reply; 4+ messages in thread
From: Christopher Li @ 2006-12-22  9:47 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: linux-sparse

On Fri, Dec 22, 2006 at 02:36:48AM -0500, Pavel Roskin wrote:
> On Fri, 2006-12-22 at 02:06 -0500, Pavel Roskin wrote:
> > It seems to me that the existing add_pre_buffer() mechanism can be used
> > instead.  I'm just a bit worried why it wasn't done like this in the
> > first place.

I believe the reason it is not in the pre buffer is that it should first
search the current directory instead of the source file directory. The command
line -include has some subtle differences with #include "filename"

> 
> There was a reason to worry.  Now create_builtin_stream() is run after
> the includes have been processed, so that e.g. the Linux compiler.h
> tells me that my compiler is too old (because it was included from the
> command line before __GNUC__ was defined).

That is the other reason as well :-)

Can you please try this patch and see if it works for you?

Chris

Index: sparse/pre-process.c
===================================================================
Index: sparse/lib.c
===================================================================
--- sparse.orig/lib.c	2006-12-05 16:17:39.000000000 -0800
+++ sparse/lib.c	2006-12-22 01:51:00.000000000 -0800
@@ -192,7 +192,10 @@ int Wdo_while = 1;
 int Wuninitialized = 1;
 int preprocess_only;
 char *include;
-int include_fd = -1;
+
+#define CMDLINE_INCLUDE 20
+int cmdline_include_nr = 0;
+struct cmdline_include cmdline_include[CMDLINE_INCLUDE];
 
 
 void add_pre_buffer(const char *fmt, ...)
@@ -262,26 +265,26 @@ static char **handle_switch_I(char *arg,
 	return next;
 }
 
-static char **handle_switch_i(char *arg, char **next)
+static void add_cmdline_include(char *filename)
 {
-	if (*next && !strcmp(arg, "include")) {
-		char *name = *++next;
-		int fd = open(name, O_RDONLY);
-
-		include_fd = fd;
-		include = name;
-		if (fd < 0)
-			perror(name);
-	}
-	if (*next && !strcmp(arg, "imacros")) {
-		char *name = *++next;
-		int fd = open(name, O_RDONLY);
+	int fd = open(filename, O_RDONLY);
+	if (fd < 0) {
+		perror(filename);
+		return;
+	}
+	if (cmdline_include_nr >= CMDLINE_INCLUDE)
+		die("too many include files for %s\n", filename);
+	cmdline_include[cmdline_include_nr].filename = filename;
+	cmdline_include[cmdline_include_nr].fd = fd;
+	cmdline_include_nr++;
+}
 
-		include_fd = fd;
-		include = name;
-		if (fd < 0)
-			perror(name);
-	}
+static char **handle_switch_i(char *arg, char **next)
+{
+	if (*next && !strcmp(arg, "include"))
+		add_cmdline_include(*++next);
+	else if (*next && !strcmp(arg, "imacros"))
+		add_cmdline_include(*++next);
 	else if (*next && !strcmp(arg, "isystem")) {
 		char *path = *++next;
 		if (!path)
@@ -623,12 +626,14 @@ static struct symbol_list *sparse_file(c
 static struct symbol_list *sparse_initial(void)
 {
 	struct token *token;
+	int i;
 
 	// Prepend any "include" file to the stream.
 	// We're in global scope, it will affect all files!
 	token = NULL;
-	if (include_fd >= 0)
-		token = tokenize(include, include_fd, NULL, includepath);
+	for (i = cmdline_include_nr - 1; i >= 0; i--)
+		token = tokenize(cmdline_include[i].filename, cmdline_include[i].fd,
+				 token, includepath);
 
 	// Prepend the initial built-in stream
 	token = tokenize_buffer(pre_buffer, pre_buffer_size, token);
Index: sparse/token.h
===================================================================
Index: sparse/lib.h
===================================================================
--- sparse.orig/lib.h	2006-12-22 01:25:52.000000000 -0800
+++ sparse/lib.h	2006-12-22 01:42:32.000000000 -0800
@@ -33,6 +33,15 @@ struct position {
 		     noexpand:1;
 };
 
+struct cmdline_include {
+	char *filename;
+	int fd;
+};
+
+extern struct cmdline_include cmdline_include[];
+extern int cmdline_include_nr;
+
+
 struct ident;
 struct token;
 struct symbol;

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

* Re: [RFC PATCH] Handling multiple -include directives
  2006-12-22  9:47   ` Christopher Li
@ 2006-12-22 22:27     ` Pavel Roskin
  0 siblings, 0 replies; 4+ messages in thread
From: Pavel Roskin @ 2006-12-22 22:27 UTC (permalink / raw)
  To: Christopher Li; +Cc: linux-sparse

Hello!

On Fri, 2006-12-22 at 01:47 -0800, Christopher Li wrote:
> On Fri, Dec 22, 2006 at 02:36:48AM -0500, Pavel Roskin wrote:
> > On Fri, 2006-12-22 at 02:06 -0500, Pavel Roskin wrote:
> > > It seems to me that the existing add_pre_buffer() mechanism can be used
> > > instead.  I'm just a bit worried why it wasn't done like this in the
> > > first place.
> 
> I believe the reason it is not in the pre buffer is that it should first
> search the current directory instead of the source file directory. The command
> line -include has some subtle differences with #include "filename"

Then maybe we need some variation of #include, e.g. #include_cmdline.
And while at that, I think -imacros should be processed slightly
differently.
 
> > There was a reason to worry.  Now create_builtin_stream() is run after
> > the includes have been processed, so that e.g. the Linux compiler.h
> > tells me that my compiler is too old (because it was included from the
> > command line before __GNUC__ was defined).
> 
> That is the other reason as well :-)
>
> Can you please try this patch and see if it works for you?

I have tried it in current (svn) MadWifi, and the output is much more
agreeable, so I think the patch is working.

However, I would prefer that we don't use fixed size arrays.  It would
be much better in the long term to use the existing "code generator".
It would be a more uniform and scalable approach.  Besides, the
generated code could be dumped for debugging purposes.

By the way, the current MadWifi is a treasure trove for anyone looking
to improve sparse.  My impression is that most if not all reported
problems are bogus.

-- 
Regards,
Pavel Roskin

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

end of thread, other threads:[~2006-12-22 22:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-12-22  7:06 [RFC PATCH] Handling multiple -include directives Pavel Roskin
2006-12-22  7:36 ` Pavel Roskin
2006-12-22  9:47   ` Christopher Li
2006-12-22 22:27     ` Pavel Roskin

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