Jim Meyering wrote: > Eric Blake wrote: > >> On 05/06/2010 12:35 PM, Jim Meyering wrote: >>> This week we noticed that a change to struct remote_error >>> was causing trouble (new member addition changed the size of >>> the object being decoded -- bad for the protocol). >>> >>> In order to ensure that no such changes sneak in again, >>> I'm planning to do the following. >>> >>> pdwtags src/libvirt_driver_remote_la-remote_protocol.o >>> >>> prints, among other things, detailed type info for every struct: >>> >>> /* 89 */ >>> struct remote_nonnull_domain { >>> remote_nonnull_string name; /* 0 8 */ >>> remote_uuid uuid; /* 8 16 */ >>> int id; /* 24 4 */ >>> >>> /* size: 32, cachelines: 1, members: 3 */ >>> /* last cacheline: 32 bytes */ >>> >>> /* BRAIN FART ALERT! 32 != 28 + 0(holes), diff = 4 */ >>> >>> }; /* size: 32 */ >> >> Ouch. Architecture sizing plays a role. On a 32-bit machine, the first >> struct is: > > There's a way out. > >> /* 86 */ >> struct remote_nonnull_domain { >> remote_nonnull_string name; /* 0 4 */ >> remote_uuid uuid; /* 4 16 */ >> int id; /* 20 4 */ >> >> /* size: 24, cachelines: 1, members: 3 */ >> /* last cacheline: 24 bytes */ >> }; /* size: 24 */ >> >> Are we sure migration between 32-bit and 64-bit hypervisors works? And >> if it does, then these structs don't quite match what is actually sent >> over the wire. At any rate, > > Not a problem. We don't send pointers over the wire. > The types are designed to be used portably. > >>> And here's a sample of its output: >>> >>> verify (sizeof (struct remote_nonnull_domain) == 32); >> >> we'd have to make this output conditional on sizeof(void*) to be >> portable to both 32- and 64- bit machines. > > Right. There are a couple options: > > - maintain two sets of sizeof-verifying "verify" directives, > one for 32-bit, the other for 64-bit. With this, the verification > is performed at compile time and requires no special tools, in general. > However, in the unusual event that we update remote_protocol.x and > need to regenerate the directives, we'll have to run pdwtags on both > i686 and x86_64. > > - maintain a textual representation of these structs and their members > (including type names, but not sizes) and make pdwtags a requirement > for running "make syntax-check", which would perform the verification. > I.e., keep a copy of the output of pdwtags, but without the comments. FYI, here's code to generate the latter: pdwtags src/libvirt_driver_remote_la-remote_protocol.o \ |perl -0777 -n \ -e 'foreach my $p (split m!\n\n/\* \d+ \*/\n!)' \ -e ' { if ($p =~ /^struct remote_/) {' \ -e ' $p =~ s!\t*/\*.*?\*/!!sg;' \ -e ' $p =~ s!\s+\n!\n!sg;' \ -e ' print "$p\n" } }' \ > remote-protocol-structs IMHO, it would be sufficient to enable a check comparing this output to a version-controlled reference file, and making it part of "make check". Of course, this would add a dependency on pdwtags/dwarves, but it would be fine/easy to skip the check on non-Linux systems. Any objection to requiring the dwarves package for development on Linux? -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list