On 12.12.2013 20:27, Eric Blake wrote: > On 12/12/2013 11:55 AM, Michal Privoznik wrote: >>> int detail; >>> -}; >>> +} ATTRIBUTE_PACKED; >> >> (What an ancient e-mail :-) ) > > 1970 was how many years before libvirt was even started? > >> >> I've raised the problem here: >> >> https://www.redhat.com/archives/libvir-list/2013-December/msg00635.html >> >> And Eric replied suggesting a fix: >> >> https://www.redhat.com/archives/libvir-list/2013-December/msg00662.html >> >> But I must say I like your approach more. Eric? > > I still think ATTRIBUTE_PACKED in a parent class is wrong; it forces the > children to be packed, and may make it impossible to implement atomic > operations on a child member that was supposed to be aligned. I'd much > rather fix the alignment in the parent class using portable constructs > than a compiler-specific non-alignment directive. > Oh right. The locking could be impossible, because if two struct members are packed in a word, e.g.: struct S { bool a; bool b; ... }; then modifying S.a requires whole struct to be fetched from mem, modifying S.a a and storing it back. However, even if our locking code to protect say S.b was written well, S.b can still get changed without lock. As S.a is being changed, another thread changes S.b, but storing S.a means overwriting S.b without lock held. I have not thought of that. So NACK then - we need Eric's approach. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list