Re: [libvirt] [PATCH 0/2]: Cleanup driver initializers

On 03/30/2009 08:09 AM, Chris Lalancette wrote: 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.