[getdns-api] getdns_return_t for destroy methods

[Paul Hoffman]

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 GETDNS_RETURN_GENERIC_ERROR
>>> 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 GETDNS_RETURN_GENERIC_ERROR.
> 
> This description feels a bit misleading to me.  

That was not at all my intention.

> If context is NULL, we can
> return an error.  

Good.

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

> We are trying to disallow destroying a valid context within an API user¹s
> 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"?

--Paul Hoffman