[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 7/7] remote generator: Annotate hyper with the actual C type



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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]