[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



2011/5/25 Eric Blake <eblake redhat com>:
> 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.

I think that being more explicit here is better and would like to
stick with annotating all hyper members. This way we cannot overlook a
missing (u)long annotation in a review because the generator will
complain for us.

Matthias


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