On 04/26/2012 11:13 AM, Christopher Li wrote:
.
On Wed, Apr 25, 2012 at 1:16 PM, Konrad Eisele<eiselekd@xxxxxxxxx> wrote:
Hello Christopher Li,
I'd like to send a patch for review to be included
into sparse. The patch builds up a dependency
tree from c parse entities downto single token,
making it possible to write tools that analyse what
token was generated by what macro and which
macros it is dependent of.
Thanks for the reply, I'm open for rewrite.
Sorry I just get back from a trip, did not have a chance to
write a reply during the travel.
I briefly look through the patch, haven't got enough time
to look at it in the detail level. Here is a few things that
jump out, I am not going to apply this patch as it is.
+void token_allocator_nofree(void)
+{
+ token_allocator.nofree = 1;
+}
What is up with this nofree thing?
There should be much better way to keep the token
unfree.
The thing is that you want to maintain the dependency
from c-parsing units downto tokens. So the struct token
should not be free'd after the parsing stage. Also
calls to free_token should actually preserve the struct
token so that it still can be referenced.
diff --git a/allocate.h b/allocate.h
index 9f1dc8c..de85320 100644
--- a/allocate.h
+++ b/allocate.h
@@ -12,6 +12,7 @@ struct allocator_struct {
struct allocation_blob *blobs;
unsigned int alignment;
unsigned int chunking;
+ unsigned int nofree;
Nack here as well.
See above.
diff --git a/attr.c b/attr.c
new file mode 100644
index 0000000..54c4c21
--- /dev/null
+++ b/attr.c
+#define HASH_LEN (1024*4)
+struct hash {
+ struct hash_v *f;
+} h[HASH_LEN];
+
--- /dev/null
+++ b/depend.c
@@ -0,0 +1,329 @@
+/*
+ * Build macros dependency tree
+ * Copyright (C) 2012 Konrad Eisele<konrad@xxxxxxxxxxx,eiselekd@xxxxxxxxx>
+ * BSD-License
+ * Redistribution and use in source and binary forms are permitted
+ * provided that the above copyright notice and this paragraph are
+ * duplicated in all such forms and that any documentation,
+ * advertising materials, and other materials related to such
+ * distribution and use acknowledge that the software was developed
+ * by the<organization>. The name of the
+ * University may not be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ */
Why BSD license? Sparse current is open software license and moving to
MIT license.
I can set in any License you want. What I dont want is
having to go through the FSF copyleft bureaucracy. BSD is
the easiest I guess, but I'll set in any text.
- struct expression *e = alloc_expression(token->pos, EXPR_STATEMENT);
+ struct expression *e = alloc_expression_tok(token, EXPR_STATEMENT);
Please don't do change like this. It does not have any add on value.
Again, to build up the dependency tree going from c-parse entities
downto tokens I need the struct token, not "pos". It doesnt change
any logic, and the changes I did to the original soucecode are minimal
that way.
The other alternative is of course to switch from "pos" to struct token
in all structures. (And retaint the tokens by default).
There are a lot more I want to comment on but I need some time to think about
what is the best way to do it. I am not oppose to adding feature to track macro
dependency, but it need to be less intrusive to the code. I want to avoid impact
the normal code path that does not care about macro dependency. e.g. the
sparse checker.
I also suspect the hashing is unnecessary. Give me some time to come up
with some suggestion.
Without the hashing quite a lot of rewrite of the internal structures
are needed, with the hashing the original structures can be maintained,
which are simple and easy.
The dependency tree caries a lot of extra data with it. "Normal"
cases dont need that. For instance you want for each token pointers to:
"Macro dependency-tree" + "Macro expansion" + "endpos" + "visited" etc.
And even on my slow laptop no difference in speed is noticable with hashing.
An middle way would be to add to each structure a "void *custom_data"
that could be used/extend by the application. Then the data structures
get a bit bigger however you dont need to traverse the hash tree.
-- Konrad
Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
