On Wed, Feb 24, 2016 at 11:58:42AM +0000, Paul Jakma wrote:
> - This seems to require various extensions to work, some seem to be GCC
> specific?
It's specific to the "GCC-like group". I have tested on:
Linux clang 3.6.1
Linux icc 14.0.3
Linux gcc 4.9.3
Linux gcc 4.7.4
Linux gcc 4.2.4
Linux gcc 3.4.6 (no, that's not a joke. I couldn't install 2.95.3 btw.)
The CI system has tested on Linux, FreeBSD 10 + 9, OpenBSD 5.8, NetBSD 7
and OmniOS (gcc). The OmniOS test actually discovered the issue where
Solaris doesn't support constructor priorities, that was fixed.
("Solaris doesn't do constructor priorities due to linker restrictions")
I have no Apple OSX system to test on. If someone could do that, that
would be appreciated. (OSX is listed as "work needed" anyway, though.)
It probably doesn't work on Visual Studio ;)
> Are there people using other compilers? (I used to regularly build
> with the Sun One Studio compiler, but I don't anymore; only clang
> and gcc).
The Sun One Studio compiler is not a supported compiler per the list in
doc/overview.texi. As far as I know, that stopped working a long time
ago. I looked at it when we set up the OmniOS CI and vaguely remember
it was infeasible to make the shipped compiler work. Since gcc is
readily available for Solaris, I don't see this as an issue.
> - The __builtin_types_{expect,compatible_p} stuff, would C11 _Generic
> allow the same to be achieved?
The __builtin_types_* stuff is actually in a commented-out section that
I forgot to remove. It was there for temporary compatibility between
the old and new schemes, so that the code works correctly even at every
single step in the middle.
> - Could the manipulation of the linked list global pointers (the
> addition of a group onto the global groups list, then additions of
> memtypes in a group onto the group list) be done behind a centralised
> function in lib/memory and go through that, rather than twiddled
> directly in each mtype specific constructor function?
Calling other functions from constructors is generally a bit of a bad
idea as the C runtime has not neccessarily [it should, but...] fully
initialized -- and any Quagga functions quite certainly won't have been
initialized. Having the code in a centralised function would make
neccessary a big fat warning on top of it so that no one accidentally
calls zlog_* or something similar. Having it right there in the header
with quite a bit of "magic" around it makes it very obvious that the
code is special.
It also provides no real gain as the struct layout is embedded into the
executable either way, i.e. there's no abstraction gain.
Lastly, on some architectures constructors are chained together as pure
function bodies without prologue and epilogue, i.e. they just run in
sequence. Moving bits to a function would nicely contravene this
"optimization" (which, honestly, I don't think makes much of a
difference, but whatever.)
> - Compiler and linker specific extensions to arrange to declare static
> stuff on by one and have a function called for each.
>
> The key thing here really is just to not have to update the library,
> right?
No -- moving memtypes into the files they're used in was also a major
design goal, thus the existence of DEFINE_MTYPE_STATIC(). A table does
not allow for that.
An alternate approach would be to use data section attributes, this
however depends on *linker* behaviour more than I'd be willing to rely
on.
[cut]
> I.e., I'm just wondering if non-standard construction extensions are
> enough of a win,
Considering that the linker functionality for constructors is
neccessarily present on any architecture that supports C++ (this is how
global variable classes get their constructors called), this boils down
to only compiler extensions. These seem to be a non-problem to the
point where I won't even call it non-standard anymore. Can you find a
compiler/environment that does NOT work (but worked before)?
> given they only eliminiate a couple of initialisation calls?
Localizing things to their point of use is in my opinion a significant
gain for both maintainability and usability; I guess the "couple of
initialisation calls" you mention refers to your suggested table-based
solution which does not provide that. So, this is apples vs. oranges.
Also, I have put considerable thought into making this as hard as
possible to (accidentally) misuse, removing for example the possibility
to forget initialisation calls. If someone forgets the DECLARE_MTYPE
(or DEFINE_MTYPE_STATIC), they get a compiler error. If someone forgets
the DEFINE_MTYPE, they get a linker error. Pass NULL as type and you
get a warning from the "nonnull" attribute. I'm not saying it's
impossible to misuse but it needs a special kind of stupid ;)
> - The formatting should follow the standard style in Quagga,
> particularly anything intended for core lib/ bits.
Sure, I can go over it again, I thought I fixed most.
-David
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev
