[getdns-api] getdns_return_t for destroy methods

Goyal, Neel ngoyal at verisign.com
Mon Mar 10 14:11:32 MST 2014


On 3/10/14, 4:55 PM, "Paul Hoffman" <paul.hoffman at vpnc.org> wrote:

>On Mar 10, 2014, at 8:31 PM, Goyal, Neel <ngoyal at verisign.com> wrote:
>
>> Hopefully an example can clarify what I mean.  Get your C hat ready.
>> Please consider the following:
>> 
>> getdns_context* context = NULL;
>> getdns_context_create(&context, 1);
>> // the context variable now points to some location in memory that
>>contains
>> // our internal context data
>> getdns_context_destroy(context);
>> // the memory that context points to has been freed up and cleared.
>> // however, the context variable still points to that memory location
>>and
>> is
>> // not NULL (context != NULL)
>> 
>> So calling getdns_context_destroy(context); again will fault because the
>> variable passed in is not NULL, and the code considers it valid.
>>There¹s
>> no way around this unless we take in a context** and set *context to
>>NULL
>> in our destructor.
>
>... or you remember all of the valid values for the pointers to contexts
>that you already destroyed.

Certainly this is an approach but also has its implications.  ³no way
around this² was a bit too absolute.  It¹s not worth going down this
rabbit hole though.

>This is feeling like you want the error for one use case ("we can see
>that they're calling the destroyer inside the callback") but are
>describing it more broadly than it deserves. If that's the case, I'm OK
>with having an error returned, but certainly not
>GETDNS_RETURN_GENERIC_ERROR: it would have to be very specific to what
>you are testing.
>
>Let me know if I have misunderstood.

No misunderstanding.  This was for a particular use/edge case.  I think if
we pull in Simon¹s thread, then we can narrow it down to two solutions
that should satisfy us:

 - introduce a return code to the destroy method that is more specific
than GETDNS_RETURN_GENERIC_ERROR
 - document this edge case / rule in the API spec and enforce it in the
implementation via asserts

I am fine with either as long as the user is made aware.




More information about the getdns-api mailing list