> That's usually solved through #define's (see e.g. lib/extable.c).

Well, you can obviously solve pretty much everything with #define's, but 
it's usually also the ugliest solution.

>From my point of view, the preferences for solving issues like the 
extable.c one are:

o Do it automatically. If the architectecture defines its own version, use
  that one -- otherwise use the default.
o Take care of it by configuration options.
  Let the arch have CONFIG_GENERIC_EXTABLE_SORT (or not), and use that to
  determine whether to compile / link sort_extable()
o Use a #define / #ifdef in the source file to conditionally compile 
  things.

Having a config option has the advantage that one can do

        obj-$(CONFIG_GENERIC_EXTABLE_SORT) += extable_sort.o

which leaves the source code free of ugly #ifdef's. Obviously that only 
works if extable.c is split into extable_sort.o and extable_search.o. If 
one goes that step (fine IMO), one can however just add these files to 
lib-y, i.e. use the first solution - everything happens automatically, no 
need for ARCH_ defines, no need for new CONFIG options.

> It's used by many filesystems plus some optional part of the USB code.
> 
> Basically, there are two choices:
> - always include a function
> - include a function only if required
> 
> The second one is possible, but object files are the wrong granularity - 
> why should it pull the whole parser.c if only match_strdup was required?

Yes, object files are not necessarily the right granularity -- in fact, 
traditionally libraries are composed of object files which pretty much 
follow a "one function per file" principle, to get this right.

However, two cases should be distinguished:

o providing a default implementation. Obviously, this one needs separate 
  files for separately needed units (like the extable.c example above)
o providing a library functionality. Grouping multiple functions together
  isn't necessarily wrong, it just may cause slight inefficencies (pulling
  in unneeded code). I really don't think we want to go to one function 
  per file but rather leave things grouped as they currently are. If 
  someone needs match_strdup, they'll pull in all of parser.o.

> You have all possibilities with #define's.

Yes, but also all the ugliness. There's a trade-off where the optimal 
smallest binary kernel image comes with a bloated, unreadable, full of 
'#ifdef' kernel source. Noone wants that. If you can optimize the binary 
kernel image in some automated way with little impact on the source code, 
that's fine. Otherwise, you'll give up a little space savings for easier 
maintainability.

> If you care about code size, CONFIG_PARSER gives the wrong granularity.

Going more fine-grained is out of the question, IMO. If anything, going 
less fine-grained, i.e. include all of parser.o unconditionally is the way 
to go here. Currently, the only more fine-grained way would be to have 
ext2 in KConfig CONFIG_MATCH_INT, CONFIG_MATCH_TOKEN. FAT would define
CONFIG_MATCH_INT, CONFIG_MATCH_TOKEN, CONFIG_MATCH_OCTAL, etc. That's 
insanity.

> Everything lib-y does can be done with obj- and #define's.
> And this way, it's done explicit.

That's true. But more often than not, we don't care about explicitness,
we care about cleanliness and readability. The ugliness is hidden 
somewhere, in includes, in Kconfig, in the build system. "module_init()" 
is very non-explicit, it does completely different things depending on 
whether a file is compiled into a module or the kernel. But we sure prefer 
the cleanliness of module_init() over having #ifdef's in each module which 
explicitly what happens.

As I said it's all a trade-off, and it has much to do with taste. Code 
looking clean is important. However, hiding too much and jumping through 
hoops just so that something looks clean while all the magic happens 
somewhere deep inside unbeknowst to all but few gurus isn't right, either.

This discussion doesn't really have much to do with the original issue 
anymore. Move parser.o to obj-y and be done with it -- in reality everyone 
needs it, anyway. EXPORT_SYMBOL() sometimes and kinda unpredictably not 
working in lib-y is tricky. But it produces obvious errors, unresolved 
symbols, not obscure un-debuggable bugs.

--Kai


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to