Re: Fwd: dependency tee from c parser entities downto token

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, May 12, 2012 at 10:46 AM, Konrad Eisele <eiselekd@xxxxxxxxx> wrote:
> Appended is a test-patch that adds test-mdep testcase.
> The file mdep.c is used to record that macro
> expansion, each token will have a reference to its
> source.
> test-mdep.c does pre-process (as test-macro.c) then
> prints out the token trace through macros for each
> token: @{ } is used to mark the active path.

Here is some feed back for the patch you send out:

>diff --git a/a.h b/a.h
>new file mode 100644

The test file should be in validation/ directory. Please give a.h a
name represent the test as well.

+               if (!strncmp(arg, "buildin", 7)) {
+                       fnobuildin = 1;
+               }

Is there a stand gcc option to handle nobuildin? I haven't found one.
If gcc has one, we can duplicate the gcc behavior. Otherwise,
I don't see it is necessary to add one. The dependence program needs
to able to handle the symbol from built in stream any way.
If it is for easier to debug, your program can trivially skip out the builtin.
stream.

+ * BSD-License
+ * Redistribution and use in source and binary forms are permitted

Can you make it cover by the sparse license file as well? I don't mind you grant
extra license to your code. But please at least make it cover by the
license of the project itself otherwise I won't apply it. It is pure overhead
to figure out what license is compatiable to spase and maintain different
file at different license. Have all file under the project cover by the same
license will keep it simple.

+void mdep_init(void) {
+    preprocess_hook = &pp;
+    pps = init_stream("<pp>", -1, 0);
+}

Please use the same coding standing as the other part of the project.
It is actually the same as linux kernel. The "{" for the function should be
on a new line. Indentation is tab and 8 char wide. There is a lot of those
little one on this file. I am not going to repeat myself here.

+struct hash {
+    struct hash_v *f;
+} h[HASH_LEN];

+struct pp {
+    enum pp_typ t;
+    union {
+        unsigned int argi;
+    };
+    struct pp_e *f;
+    struct symbol *sym;
+    struct token *tok;
+    struct token *s, *d;
+};
+

I hate those one letter member name. You have to guess what it is
all the time. pp_dope_list() is full of those one letter variable name as
well, make it very hard to read. The variable name need to be a little
bit more meaningful.


+       show_tokenstream(token, 0);
Please don't use 0 for NULL pointer.

-void show_tokenstream(struct token *token)
+void show_tokenstream(struct token *token, struct token *end)
-       while (!eof_token(token)) {
+       while (token != end && !eof_token(token)) {

I don't see you use the show_tokenstream with two arguments.
You write your own show_tokenstream for mdep any way.
You can leave it one unchanged.

@@ -194,7 +196,7 @@ void show_tokenstream(struct token *token)
-               printf("%s%.*s", show_token(token), prec, separator);
+               fprintf(stderr,"%s%.*s", show_token(token), prec, separator);

Can't do that, the "-E" will use show_tokenstream() as well. You just
change it to stderr for "-E".


To be continue...

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


[Index of Archives]     [Newbies FAQ]     [LKML]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Trinity Fuzzer Tool]

  Powered by Linux