[getdns-api] getdns-api review - section 2.1

[Goyal, Neel]

On 1/29/14, 4:34 PM, "Paul Hoffman" <paul.hoffman at vpnc.org> wrote:

>On Jan 29, 2014, at 12:45 PM, Goyal, Neel <ngoyal at verisign.com> wrote:
>
>> Hi all,
>> 
>> This is commentary on section 2.1 in reference to Bob Steagall¹s review
>>of the getdns API -
>>http://www.vpnc.org/pipermail/getdns-api/2014-January/000229.html
>> 
>> The suggestion is to use enumerated types where possible.  In
>>particular, getdns_return_t and some context setter function parameters.
>> An example would be:
>> 
>> 
>> getdns_return_t getdns_context_set_resolution_type( getdns_context_t
>>context, uint16_t value );
>> 
>> 
>> The spec says the value must be either GETDNS_CONTEXT_RECURSING or
>>GETDNS_CONTEXT_STUB.
>> 
>> I am in favor of this change - definitely for getdns_return_t.  The
>>other functions will be a bit of work, but I think they do result in a
>>cleaner API in the long run.
>> 
>> Thoughts?
>
>During the original design, the sentiment was against enumerateds for
>these types of lists because an implementation didn't know how big to
>make them. If we have a stated rule that all enumerated values will be
>less than 64K, that makes the implementations a bit more bounded.

I believe that we should use enumerated types where the API Specification
says that the values are invalid unless they are one of the values defined
in the header (I.e. GETDNS_CONTEXT_RECURSING, GETDNS_CONTEXT_STUB for
resolution type, GETDNS_CONTEXT_APPEND_NAME_ALWAYS,
GETDNS_CONTEXT_APPEND_NAME_ONLY_TO_SINGLE_LABEL_AFTER_FAILURE,GETDNS_CONTEX
T_APPEND_NAME_ONLY_TO_MULTIPLE_LABEL_NAME_AFTER_FAILURE, or
GETDNS_CONTEXT_DO_NOT_APPEND_NAMES for append_name, etc.

Note that the functions could become something like:

getdns_return_t
getdns_context_set_dns_transport(
  getdns_context_t       context,
  getdns_transport_t value
);


getdns_return_t
getdns_context_set_append_name(
  getdns_context_t       context,
  getdns_append_name_t value
);


Etc.

All possible values do not end up in one giant enum list, rather each
option type gets their own. I¹m not sure what is meant by the
"implementation didn't know how big to make them² - for some it is clear
based on the specification language.  For instances where we want to add
flexibility, then the implementation either adds extra options, or we
change it to a generic type (like it is now).  Do we see the need to keep
all of these setters flexible at the expense of type safety?