On Wed, May 25, 2011 at 10:33:06AM -0600, Eric Blake wrote: > On 05/25/2011 10:23 AM, Daniel P. Berrange wrote: > > On Mon, May 23, 2011 at 07:36:10PM +0200, Matthias Bolte wrote: > >> Remove some special case code that took care of mapping hyper to the > >> correct C types. > >> > >> Use macros for hyper to long assignments that perform overflow checks > >> when long is smaller than hyper. Also use such macros for the safe > >> hyper to longlong assignemts as this allows to keep the generator a > >> bit simpler. > > > >> + } elsif ($ret_member =~ m/^(unsigned )?hyper (\S+)\[\S+\];/) { > >> + # error out on unannotated hypers > >> + die "(unsigned)? hyper without (u)?(long|longlong) annotation: > >> $ret_member"; > > > > 'hyper' in XDR world is a fixed 64 bit type, so IMHO, this > > should automatically map to 'long long' in the API, without > > requiring an annotation. > > > > eg, we should only annotate if the public API uses the variable > > sized 'long' / 'unsigned long' types in C. > > > All the 'longlong' and 'ulonglong' annotations should > > be removed here IMHO, since they should be the default > > for hyper/unsigned hyper. > > If we add a new API that uses 'long' but forget the annotation, then we > have silent type mismatch in the generated code. Since we are using > hyper for both types in C, I find it better to require an annotation on > all uses to be explicit on which type we meant, rather than defaulting > the lack of annotation as 'long long' and only requiring annotation for > 'long' - that is, I find that from the maintenance aspect, having the > code generator explicitly fail because you forget an annotation (even if > the annotation is the default of llong) is safer than risking silent > code misgeneration. > > But anything is better than nothing, so I won't give any further > complaints if we go with your idea of only annotating the exceptions to > the 'long long' default, rather than all uses of hyper.
My rationale for defaulting to 'long long' is that we should never add any more APIs which take a plain 'long' anywhere. It would be a nice idea if we could get apibuild.py or something to raise an error if it finds any more APIs using 'long', except for a whitelist of existing ones Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list