[getdns-api] getdns_return_t for destroy methods
ngoyal at verisign.com
Mon Mar 10 13:31:37 MST 2014
On 3/10/14, 4:15 PM, "Paul Hoffman" <paul.hoffman at vpnc.org> wrote:
>On Mar 10, 2014, at 5:15 PM, Goyal, Neel <ngoyal at verisign.com> wrote:
>>>> The implementation can not. But, the application using the
>>>> implementation/library *can* do it from within a callback, because the
>>>> callbacks are part of that application.
>>>> What we propose, is to let a getdns_context_destroy, when used from
>>>> within a callback, return GETDNS_RETURN_GOOD if the
>>>> library/implementation can deal with it, and
>>>> if it can not.
>>>> Our implementation currently deals with it, but it is administratively
>>>> cumbersome (implementation wise) and therefore susceptible for bugs in
>>>> the future. For robustness we would rather disallow it.
>>> OK, that makes more sense. How about this instead:
>>> getdns_return_t getdns_context_destroy(struct getdns_context* context);
>>> Returns destroys the context and GETDNS_RETURN_GOOD if the context
>>> exists. If the context does not exist (for example, because it was
>>> previously destroyed), the function returns
>> If context is already destroyed and not NULL, the code
>> will crash.
>So you do not check whether or not the "context" argument is an argument
>you know about?
>> This is analogous to the double free problem that exists in C.
>Sure, but I thought that was why we had these constructors and
>destructors, but maybe not.
Hopefully an example can clarify what I mean. Get your C hat ready.
Please consider the following:
getdns_context* context = NULL;
// the context variable now points to some location in memory that contains
// our internal context data
// the memory that context points to has been freed up and cleared.
// however, the context variable still points to that memory location and
// 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. I don’t recommend that since C programmers will be
used to the behavior in the current API.
>> We are trying to disallow destroying a valid context within an API
>> asynchronous callback. How about the last sentence becomes "If the
>> context cannot be destroyed (for example, within the body of a
>> getdns_callback_t), the function returns GETDNS_RETURN_GENERIC_ERROR.".
>Wouldn't a context that could not be destroyed and not NULL also be "a
>context that cannot be destroyed"?
Yes - did I miss something? I wanted to clarify the “context does not
exist (for example, because it was previously destroyed)” portion.
More information about the getdns-api