On 03/30/2009 08:09 AM, Chris Lalancette wrote:
All,
This is a small series to cleanup the internal driver initializers. The
drivers were using an annoying combination of C-99 style initialization and
old-style (C89?) initialization. On top of that, the indentation on some of
these was wacky, making it even harder to read. After some discussion with
DanB, we determined to go with the old-style initializers because it is sort of
a self-documenting TODO list.
I see your point about the TODO list (since the old style initializers
will throw a compiler error if you add a field to a struct and fail to
initialize it everywhere, forcing you to at least put a "NULL," line
in), while using the new style causes all unspecified fields to be
auto-initialized to 0/NULL), but wanted to share my experience a couple
weeks ago when attempting to understand the code by tracing through by
hand (well, with emacs and xcscope.el ;-). I actually changed some
struct initializations in my local directory in the opposite direction -
from old style to new style (called "designated initializers", I found
out). I did this because it makes it *much* easier to find your way if
you're using something like cscope, which ignores comments.
As an example, when trying to figure out how virNetworkCreateXML works,
my first step was to look at that function and see that it calls
conn->networkDriver->networkCreateXML, which is a function pointer in
virnetworkDriver. So, to see what actual code that might lead to, I put
the cursor over networkCreateXML, hit and it finds:
*** src/driver.h:
<global>[480] virDrvNetworkCreateXML networkCreateXML;
*** src/remote_internal.c:
<global>[6907] .networkCreateXML =
remoteNetworkCreateXML,
*** src/libvirt.c:
virNetworkCreateXML[4826] if (conn->networkDriver &&
conn->networkDriver->networkCreateXML) {
virNetworkCreateXML[4828] ret =
conn->networkDriver->networkCreateXML (conn, xmlDesc);
You notice that it finds the place where networkCreateXML is initialized
to remoteNetworkCreateXML, because a deisgnated initializer was used
there. What's missing, though, is the initialization in
network_driver.c, where the old style is used (cscope doesn't index
comments, and comments aren't reliable anyway - during my explorations
I've found cut-pasted code that had unmodified pre-cut-paste comments
that are incorrect).
Once I change the initialization of the virNetworkDriver struct in
network_driver.c to use designated initializers, cscope can find the
line, and I get a better picture of how the code works.
*** src/network_driver.c:
<global>[1419] .networkCreateXML = networkCreate,
Of course this is just one example, but I'm guessing the same would
happen to other people in the same situation.
Another advantage I will point out to designated initializers is that
there is never any chance of inadvertantly mixing up the order of the
members of a struct when initializing an instance (very easy to do when
many of the fields are initialized to NULL), and thus introducing a bug
that is impossible to spot during a simple visual inspection.
On the down side of designated initializers (and it seems the big reason
you chose to standardize on the old style), since structures instances
that are initialized with designated initializers don't require
explicitly setting each and every member (unspecified members are just
filled with 0's), it's possible to leave something out, and not receive
a complaint from the compiler. (Wouldn't it be nice if that was added to
gcc? Then everyone could get what they wanted at the same time)
A partial bandaid to using old-style is to (as you've done) put the
member name in a comment on the same line as the initial value. This
won't help cscope, and isn't compiler-guaranteed to be correct, but at
least tools (other than cscope :-() will find it. I would strongly
encourage anyone using old-style initializers to follow this practice.
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list