On Sun, 2009-05-31 at 21:31 -0400, Duane Ellis wrote: > duane> [about target_types.h] and "openocd/types.h" > > I do like what you have done with "target_types.h" - Yes, absolutely - > good idea, I think I said it wrong, I'm sure I'll make that mistake again. > > zach> You want to expose every struct name to every module? No thanks. > > We do that now - in a messy way, note: i say "struct name" not "struct > content" > > zach> we would not need such forward declarations if if we used the > 'struct foo_s *' form instead of the typedefs. > > Absolutely agree with you. I *specifically* do not mean the *contents* > of a structure, absolutely not. I'm trying to stop a rash of warnings > like this: > > a.c:2: warning: "struct abcXXX" declared inside parameter list > a.c:2: warning: its scope is only this definition or declaration, which > is probably not what you want
Hmmm... I was trying to figure out why I am not seeing these with my first patch (jtag-tap-1.patch). > The simple workaround for the above is, a simple forward struct > declaration, nothing more, as in (1) below. > > (1) struct abcXXX; > (2) int foobar( struct abcXXX *p ); > > Either (A) - is done via a cascade of #include files - effectively what > we have today, #include <STAR.dot.STAR>, and is shown below via "arm11.c" That simply is not true. I spent a lot of time cleaning up things, and the tree of headers that you show below is a _gross_ improvement on what was going on before. I can improve this mess further, and without using a common header like you suggest. > or (B) every H file has it's own little 'struct foo; /* forward > declaration */ - This devolves over time into voodoo and countless > useless 'struct foo' things added all over the place, randomly - some > files have them, some don't, there is no rhyme nor reason why, suddenly > a file one needs it, and the next one does not. They look out of place, > "people clean up" - (like we are doing today) Given some time, these > devolve into a mess I disagree with this assessment. This is not voodoo. These are simple forward declarations that are much clearer that a typedef or forward declaration that has been hidden away in another file. > or (C) you have a simple project specific "types.h" file - that contains > all the simplistic basic types used in the project, and that may include > a few major "struct foo;" names in name only (NOT with content of that > struct), the moral equal to "sys/types.h", it is a simple solution. > Yea, a little messy, but a reasonable mess to contain. Simple rule: (a) > always put a forward declaration of any structure in "openocd_types.h", > and (b) put the structure where it belongs, thus any function can "get a > pointer to the object" - but that is all. > > (C) simplifies and streamlines the process, a simple "openocd_types.h" > file - is always included, like it is today. And it also completely flattens the module hierarchy, crushing the ideas of modularity and API layering. I strenuously object to this proposal. > ---- > For instance, I picked arm11.c - at random, then number in column 1 is > the nesting level. > > arm11.c > 1 #include "arm11.h" > 2 #include "target.h" > 3 Extra 'struct reg_s' > 3 Extra 'struct trace_s' > 3 Extra 'command_context_s' > 3 Extra "struct target_type_s' > 3 Extra 'struct target_s' > > 3 #include "breakpoints.h" > 4 Extra "struct target_s" > 4 #include "type.h" > > 3 #include "algorithm.h" > 4 #include "type.h" > > 3 #include "command.h" > 4 #include "types.h" > 4 #include "jim.h" > > 2 #include "register.h" > 3 Extra 'stuct target_s' > > 3 #include "types.h" > > 2 #include "jtag.h" > 3 #include "binarybuffer.h" > 4 #include "types.h" > 3 #include "log.h" > 4 #include "command.h" > > ----- > > That mess goes away with a simple "openocd_types.h", the moral equal to > "sys/types.h" - but in this case, a bunch of "forward struct" > definitions. I am not suggesting that the contents of structs be put in > this file. I think that tree is beautiful compared to what it used to be. Seriously, you need to go back and try your experiment with the tree *before* I made the massive header file cuts. It would not look as pretty in an e-mail message. Those declarations that you label as "extra" are _not_. They allow other source files than the one you illustrated to include the header files in question _without_ also pulling in the header that defines the related structure. They decouple the modules more completely. Cheers, Zach _______________________________________________ Openocd-development mailing list [email protected] https://lists.berlios.de/mailman/listinfo/openocd-development
