Messages in this thread | | | Date | Wed, 24 Jul 2002 16:40:54 +0400 | From | Joshua MacDonald <> | Subject | Re: type safe lists (was Re: PATCH: type safe(r) list_entry repacement: generic_out_cast) |
| |
On Wed, Jul 24, 2002 at 02:22:32PM +0200, Jakob Oestergaard wrote: > On Wed, Jul 24, 2002 at 02:07:45AM +0400, Joshua MacDonald wrote: > ... > > This may interest you. We have written a type-safe doubly-linked list > > template which is used extensively in reiser4. This is the kind of thing that > > some people like very much and some people hate, so I'll spare you the > > advocacy. > > Ok, here's my comments: > > *) Using macros like that is ugly as hell, but as I see it it is the > only way to achieve type safety in such generic code. If the ugliness > is confined to one header file, and possibly one .c file containing the > needed instantiations, I would argue that the ugliness is bearable. In > other words, I think your solution is the prettiest one possible. > > Since all list routines right now are extremely simple, it's probably ok > to just have it all in a header. If larger routines are added later on, > it may be desirable to create a .c file holding the needed (macro > instantiated) routines. In that case, the following applies: > > *) I would suggest making one list_instances.c which holds all the > INSTANTIATE... definitions of the list types needed in the kernel. This > way we will avoid having two list codes generated for the same type (an > easy accident to make with the macro approach) > *) You would have to somehow separate the "simple" routines which should > be inlined, and the larger ones which should remain function calls. This > would mean that a .c file using the list header would have the inline > functions declared in the list header (using static inline so unused > routines won't bloat the .o), and find it's larger out-of-line routines > in the global list .o. > > The reason I'm suggesting doing the above instead of simply > instantiating a list implementation for each type needed, whenever it is > needed, is simply to avoid code bloat. > > My only comment for the code would be to require that the link from the > element into the list would always be called "rx_list_backlink" or > whatever (if the list name is "rx_list") - the freedom you have in > specifying what the LINK_NAME is, is useless as I see it, and only adds > to the confusion.
Jakob,
These are fine suggestions. The debug-only list invariants, the splice function, and possibly others are definetly candidates for non-static-inline inclusion. A single list_instances.c file makes a lot of sense (except for modules--maybe?), and you are right that LINK_NAME is basically useless.
Since this code is already part of reiser4, it is likely to be crititically reviewed again when Hans begins his push. I will fix the LINK_NAME issue and change the reiser4-specific assert() calls to generic ones. We welcome feedback.
-josh
> -- > ................................................................ > : jakob@unthought.net : And I see the elder races, : > :.........................: putrid forms of man : > : Jakob Østergaard : See him rise and claim the earth, : > : OZ9ABN : his downfall is at hand. : > :.........................:............{Konkhra}...............: - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |