Re: integration of c-ares into various file descriptor based main loops

2013-11-07 Thread Alexander Klauer
On 11/07/2013 01:44 PM, Daniel Stenberg wrote:
> On Wed, 6 Nov 2013, Jakub Hrozek wrote:
> 
>> Maybe you could also include libverto. It's relatively new event loop
>> library that encapsulates other event loop approaches as back ends.
>> It's being used by MIT Kerberos and GSS-Proxy among others.
> 
> I would say that the most sensible thing for a library to do is to
> provide callbacks for what file descriptors to wait for (ADD, REMOVE,
> MODIFY) activities on, and what those activities are. And a separate one
> that sets a timeout timer if necessary.
> 
> That should be possible to make work with any event library or select()
> / poll() or whatever out there.
> 
> Not completely out of coincidense, that's the API libcurl provides for
> doing event-based operations...

I concur, a library should not bind itself to a specififc event handling
framework, especially because two different approaches in the same
application may be difficult to reconcile with each other.

But, since you mentioned timeouts, I start wondering whether it is such
a good idea to communicate only one single timeout. Doing so means you
have to maintain a priority queue of timeouts somewhere under the hood,
which is a complex thing. So, why not simply let the users know the
timeout for each fd, so they can put it in some priority queue (such as
the one provided by the event loop library) together with all the other
stuff that goes in there (one timeout queue per application).

This also lets the users make the design choices the library author does
not have enough information to make.

Best regards,
Alexander


Re: [PATCH 2/3] library init: documentation update

2013-04-22 Thread Alexander Klauer

Hi,

On 03/18/2013 09:26 AM, Alexander Klauer wrote:

OK, I've updated the commit.


sorry for being a pest, but this patchset still has not made it into the 
master branch at https://github.com/bagder/c-ares/commits/master.


Is there still something wrong with it? What can I do to help?


Best regards,
Alexander


Re: [PATCH 2/3] library init: documentation update

2013-04-10 Thread Alexander Klauer

Hi Günter,

On 04/10/2013 12:44 PM, Guenter wrote:

Hi Alexander,
On 10.04.2013 10:03, Alexander Klauer wrote:

Any further comments? If not, could this patchset be committed, please?

from what I see your patches are already committed:
https://github.com/bagder/c-ares/commits/master


on that page, I see only my ares_cancel() patchset, not the 
ares_library_init()/_cleanup() one.


Best regards,
Alexander


Re: [PATCH 2/3] library init: documentation update

2013-04-10 Thread Alexander Klauer

On 04/10/2013 11:07 AM, Alexander Klauer wrote:

Hi,

no need to CC me, I'm subscribed.

On 04/10/2013 10:44 AM, Saúl Ibarra Corretgé wrote:

On 4/10/13 10:03 AM, Alexander Klauer wrote:

On 03/18/2013 09:26 AM, Alexander Klauer wrote:

On 03/15/2013 08:26 PM, Yang Tse wrote:

On Fri, Mar 15, 2013, Alexander Klauer
 wrote:


This commit updates the documentation of ares_library_init() and
ares_library_cleanup() with regard to the new recursive behaviour.


It might be better to use "Its reference counted
initialize/deinitialize behavior," or something similar, instead of
"Its recursive behavior,"



OK, I've updated the commit.


Any further comments? If not, could this patchset be committed, please?
TIA.



I'm no expert, but I thought I'd throw my 2 cents here :-) What about
using something like pthread_once? Would that work for you? Not sure if
that's supported in all platforms though.


That would work on systems where it's supported, but unfortunately, it's
not portable. Since one of the points of having an asynchronous resolver
is not having to deal with threads in the first place, I'd be loath to
import a dependency on a threading library into c-ares.


I just forgot: pthread_once() would work for the library init, but the 
finalisation has to be taken care of as well, so you need some kind of 
reference counting.


Best regards,
Alexander


Re: [PATCH 2/3] library init: documentation update

2013-04-10 Thread Alexander Klauer

Hi,

no need to CC me, I'm subscribed.

On 04/10/2013 10:44 AM, Saúl Ibarra Corretgé wrote:

On 4/10/13 10:03 AM, Alexander Klauer wrote:

On 03/18/2013 09:26 AM, Alexander Klauer wrote:

On 03/15/2013 08:26 PM, Yang Tse wrote:

On Fri, Mar 15, 2013, Alexander Klauer
 wrote:


This commit updates the documentation of ares_library_init() and
ares_library_cleanup() with regard to the new recursive behaviour.


It might be better to use "Its reference counted
initialize/deinitialize behavior," or something similar, instead of
"Its recursive behavior,"



OK, I've updated the commit.


Any further comments? If not, could this patchset be committed, please?
TIA.



I'm no expert, but I thought I'd throw my 2 cents here :-) What about
using something like pthread_once? Would that work for you? Not sure if
that's supported in all platforms though.


That would work on systems where it's supported, but unfortunately, it's 
not portable. Since one of the points of having an asynchronous resolver 
is not having to deal with threads in the first place, I'd be loath to 
import a dependency on a threading library into c-ares.


Best regards,
Alexander


Re: [PATCH 2/3] library init: documentation update

2013-04-10 Thread Alexander Klauer

On 03/18/2013 09:26 AM, Alexander Klauer wrote:

On 03/15/2013 08:26 PM, Yang Tse wrote:

On Fri, Mar 15, 2013, Alexander Klauer
 wrote:


This commit updates the documentation of ares_library_init() and
ares_library_cleanup() with regard to the new recursive behaviour.


It might be better to use "Its reference counted
initialize/deinitialize behavior," or something similar, instead of
"Its recursive behavior,"



OK, I've updated the commit.


Any further comments? If not, could this patchset be committed, please? TIA.

Best regards,
Alexander


[PATCH 1/4] ares_cancel(): cancel requests safely

2013-04-08 Thread Alexander Klauer
An invocation of ares_cancel() walks through the request list, calling
the callbacks of all pending requests on a channel. Previously, if such
a callback added a new request to the channel, the request list might
not end up empty, causing an abort by assertion failure. The present
commit ensures that precisely all requests present upon entry of
ares_cancel() are cancelled, and that adding new requests through
callbacks is safe.
---
 ares_cancel.c |   40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/ares_cancel.c b/ares_cancel.c
index e5bb050..465cc9e 100644
--- a/ares_cancel.c
+++ b/ares_cancel.c
@@ -26,33 +26,33 @@
 void ares_cancel(ares_channel channel)
 {
   struct query *query;
+  struct list_node list_head_copy;
   struct list_node* list_head;
   struct list_node* list_node;
   int i;
 
-  list_head = &(channel->all_queries);
-  for (list_node = list_head->next; list_node != list_head; )
+  if (!ares__is_list_empty(&(channel->all_queries)))
   {
-query = list_node->data;
-list_node = list_node->next;  /* since we're deleting the query */
-query->callback(query->arg, ARES_ECANCELLED, 0, NULL, 0);
-ares__free_query(query);
-  }
-#ifndef NDEBUG
-  /* Freeing the query should remove it from all the lists in which it sits,
-   * so all query lists should be empty now.
-   */
-  assert(ares__is_list_empty(&(channel->all_queries)));
-  for (i = 0; i < ARES_QID_TABLE_SIZE; i++)
-{
-  assert(ares__is_list_empty(&(channel->queries_by_qid[i])));
-}
-  for (i = 0; i < ARES_TIMEOUT_TABLE_SIZE; i++)
+/* Swap list heads, so that only those queries which were present on entry
+ * into this function are cancelled. New queries added by callbacks of
+ * queries being cancelled will not be cancelled themselves.
+ */
+list_head = &(channel->all_queries);
+list_head_copy.prev = list_head->prev;
+list_head_copy.next = list_head->next;
+list_head_copy.prev->next = &list_head_copy;
+list_head_copy.next->prev = &list_head_copy;
+list_head->prev = list_head;
+list_head->next = list_head;
+for (list_node = list_head_copy.next; list_node != &list_head_copy; )
 {
-  assert(ares__is_list_empty(&(channel->queries_by_timeout[i])));
+  query = list_node->data;
+  list_node = list_node->next;  /* since we're deleting the query */
+  query->callback(query->arg, ARES_ECANCELLED, 0, NULL, 0);
+  ares__free_query(query);
 }
-#endif
-  if (!(channel->flags & ARES_FLAG_STAYOPEN))
+  }
+  if (!(channel->flags & ARES_FLAG_STAYOPEN) && 
ares__is_list_empty(&(channel->all_queries)))
   {
 if (channel->servers)
 {
-- 
1.7.9.5



[PATCH 4/4] .gitignore: ignore patch files

2013-04-08 Thread Alexander Klauer
This commit adds a line to .gitignore to the effect that patch files
generated by 'git format-patch' are excluded from the repository.
---
 .gitignore |1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 19a0eae..0b1087a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -82,3 +82,4 @@ ares_timeout.pdf
 ares_version.pdf
 c-ares-*.tar.gz.asc
 ares_parse_mx_reply.pdf
+/[0-9]*.patch
-- 
1.7.9.5



[PATCH 2/4] Documentation: properly document ARES_ECANCELLED

2013-04-08 Thread Alexander Klauer
This commit clarifies the behaviour of ares_cancel() with respect to
callbacks and adds missing documentation of ARES_ECANCELLED to the man
pages of the affected functions.
---
 ares_cancel.3|4 +++-
 ares_gethostbyaddr.3 |3 +++
 ares_gethostbyname.3 |3 +++
 ares_getnameinfo.3   |3 +++
 ares_query.3 |3 +++
 ares_search.3|3 +++
 ares_send.3  |3 +++
 7 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/ares_cancel.3 b/ares_cancel.3
index ef10ec5..0bf6281 100644
--- a/ares_cancel.3
+++ b/ares_cancel.3
@@ -28,7 +28,9 @@ name service channel identified by \fIchannel\fP.  
\fBares_cancel\fP invokes
 the callbacks for each pending query on the channel, passing a status of
 .BR ARES_ECANCELLED .
 These calls give the callbacks a chance to clean up any state which
-might have been stored in their arguments.
+might have been stored in their arguments. If such a callback invocation adds
+a new request to the channel, that request will \fInot\fP be cancelled by the
+current invocation of \fBares_cancel\fP.
 .SH SEE ALSO
 .BR ares_init (3)
 .BR ares_destroy (3)
diff --git a/ares_gethostbyaddr.3 b/ares_gethostbyaddr.3
index f589868..7727307 100644
--- a/ares_gethostbyaddr.3
+++ b/ares_gethostbyaddr.3
@@ -70,6 +70,9 @@ was not found.
 .B ARES_ENOMEM
 Memory was exhausted.
 .TP 19
+.B ARES_ECANCELLED
+The query was cancelled.
+.TP 19
 .B ARES_EDESTRUCTION
 The name service channel
 .I channel
diff --git a/ares_gethostbyname.3 b/ares_gethostbyname.3
index a578a53..6b24ea4 100644
--- a/ares_gethostbyname.3
+++ b/ares_gethostbyname.3
@@ -74,6 +74,9 @@ was not found.
 .B ARES_ENOMEM
 Memory was exhausted.
 .TP 19
+.B ARES_ECANCELLED
+The query was cancelled.
+.TP 19
 .B ARES_EDESTRUCTION
 The name service channel
 .I channel
diff --git a/ares_getnameinfo.3 b/ares_getnameinfo.3
index 7e4990c..d227606 100644
--- a/ares_getnameinfo.3
+++ b/ares_getnameinfo.3
@@ -109,6 +109,9 @@ was not found.
 .B ARES_ENOMEM
 Memory was exhausted.
 .TP 19
+.B ARES_ECANCELLED
+The query was cancelled.
+.TP 19
 .B ARES_EDESTRUCTION
 The name service channel
 .I channel
diff --git a/ares_query.3 b/ares_query.3
index 0c5df84..733fbc9 100644
--- a/ares_query.3
+++ b/ares_query.3
@@ -118,6 +118,9 @@ No name servers could be contacted.
 .B ARES_ENOMEM
 Memory was exhausted.
 .TP 19
+.B ARES_ECANCELLED
+The query was cancelled.
+.TP 19
 .B ARES_EDESTRUCTION
 The name service channel
 .I channel
diff --git a/ares_search.3 b/ares_search.3
index 4184b00..2c85d20 100644
--- a/ares_search.3
+++ b/ares_search.3
@@ -119,6 +119,9 @@ No name servers could be contacted.
 .B ARES_ENOMEM
 Memory was exhausted.
 .TP 19
+.B ARES_ECANCELLED
+The query was cancelled.
+.TP 19
 .B ARES_EDESTRUCTION
 The name service channel
 .I channel
diff --git a/ares_send.3 b/ares_send.3
index 48d90ab..b89abfe 100644
--- a/ares_send.3
+++ b/ares_send.3
@@ -73,6 +73,9 @@ No name servers could be contacted.
 .B ARES_ENOMEM
 Memory was exhausted.
 .TP 19
+.B ARES_ECANCELLED
+The query was cancelled.
+.TP 19
 .B ARES_EDESTRUCTION
 The name service channel
 .I channel
-- 
1.7.9.5



[PATCH 3/4] ares_destroy() documentation: no new requests

2013-04-08 Thread Alexander Klauer
Clarify that no new requests may be added to a resolver channel that is
currently being destroyed.
---
 ares_destroy.3 |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ares_destroy.3 b/ares_destroy.3
index 79171ec..3724df1 100644
--- a/ares_destroy.3
+++ b/ares_destroy.3
@@ -33,7 +33,8 @@ invokes the callbacks for each pending query on the channel, 
passing a
 status of
 .BR ARES_EDESTRUCTION .
 These calls give the callbacks a chance to clean up any state which
-might have been stored in their arguments.
+might have been stored in their arguments. A callback must not add new
+requests to a channel being destroyed.
 .SH SEE ALSO
 .BR ares_init (3),
 .BR ares_cancel (3)
-- 
1.7.9.5



Re: [PATCH 1/4] ares_cancel(): ensure cancellation of all requests

2013-04-08 Thread Alexander Klauer

On 03/23/2013 03:12 PM, Daniel Stenberg wrote:

On Thu, 21 Mar 2013, Alexander Klauer wrote:


Implementation-wise, I think I would just swap the list head in the
ares channel with an empty list head on the stack, and then start
walking through the list with the list head on the stack. Any new
requests would then be added to the now initially empty list in the
ares channel.


I think this sounds like a really sane approach that is uncomplicated,
gives no surprises to a user and is easy to document as well.


OK then, I'll implement it this way.

Alexander


Re: [PATCH 1/4] ares_cancel(): ensure cancellation of all requests

2013-03-21 Thread Alexander Klauer

On 03/21/2013 02:56 PM, Tommie Gannert wrote:

2013/3/21 Alexander Klauer :

What happens if someone calls ares_cancel() while ares_cancel() is in
progress? With (1), that would mean that the topmost (most recent) instance
ares_cancel() has to cancel all those requests that appeared since the
next-to-topmost instance was invoked, right?


Good corner case.

I try to think of how I would expect it to work if it was just
multiple threads doing
requests and ares_cancel() would cancel them. And I think what you suggest
makes a lot of sense. The second call would be responsible for cancelling all
active requests though, so if you have three requests, A B C, and cancelling A
causes a request D to be created, and then B calls cancel(), then you would
have a request list like "C D" when entering that second cancel(). In that case
both the first and second cancel() would be responsible for cancelling C, and
it would have to be cancelled before the second cancel() returns (and implicitly
thus also the first as they were recursive calls.)

Might be tricky to get right without ref counting. If it requires a lot of code
changes, maybe it's not worth enforcing such strict semantics, though.


Implementation-wise, I think I would just swap the list head in the ares 
channel with an empty list head on the stack, and then start walking 
through the list with the list head on the stack. Any new requests would 
then be added to the now initially empty list in the ares channel.


In your example, the first ares_cancel() would cancel A, B, and C, while 
the second ares_cancel() would cancel only D. This approach also works 
with more than two ares_cancel()s and no reference counting is needed.



No real use case, my leaning towards (2) is just personal feeling. My actual
use case right now is to call ares_cancel() immediately prior to
ares_destroy(). With the implementation of (1), I would of course simply
call ares_destroy(). I also call ares_cancel() in an out of memory
condition.


Then it sounds like the proposed functionality would actually benefit your code?


It would save one statement, yes.


Best regards,
Alexander


Re: [PATCH 1/4] ares_cancel(): ensure cancellation of all requests

2013-03-21 Thread Alexander Klauer

On 03/21/2013 01:44 PM, Tommie Gannert wrote:

2013/3/21 Alexander Klauer :

How about this:

* Make ares_destroy() more user friendly and allow callbacks to register new
requests on the channel being destroyed. Those requests would then
immediately be finished with ARES_EDESTRUCTION.
* Let ares_cancel() walk through the list, but don't assert it's empty
afterwards.


+1 on both accounts, but I guess that doesn't fully solve your issue. As a user
of the library, I would think that semantic is sound. Possibly with the addition
that ares_cancel() should not (accidentally) cancel any new requests created
within it, if that could happen.


Sure.

What happens if someone calls ares_cancel() while ares_cancel() is in 
progress? With (1), that would mean that the topmost (most recent) 
instance ares_cancel() has to cancel all those requests that appeared 
since the next-to-topmost instance was invoked, right?



Then, to get behaviour (1), one would call ares_cancel(), and for behaviour
(2), you would use ares_destroy(), followed by ares_init(). A little awkward
perhaps, but the API would not be broken.

Thoughts?


I'm curious what your use case is, and I agree it's awkward to have to use
ares_destroy() if you don't really mean to destroy it.


No real use case, my leaning towards (2) is just personal feeling. My 
actual use case right now is to call ares_cancel() immediately prior to 
ares_destroy(). With the implementation of (1), I would of course simply 
call ares_destroy(). I also call ares_cancel() in an out of memory 
condition.


Come to think of it, ares_cancel() is pretty useless. Normally, you 
would either want to cancel one single request, the functionality for 
which is missing, or cancel everything when you destroy the channel.



*Thinking out loud* Could one perhaps add a channel flag that would alter
the behaviour of ares_cancel()? Basically a boolean disabling the whole
library. Does that make sense? I don't know of any other library doing that,
except for "shutdown," which is what we/you would like to avoid.


I think you mean the whole channel, not the whole library. I'm not sure 
that's a good idea. All the other public flags are about what ares does 
with stuff sent and received over the network, not internal behaviour. 
shutdown() has a specific use in the case of TCP connections (and it 
does get its own flags). OTOH, my current implementation of (2) uses 
just that flag variable internally.



Best regards,
Alexander


Re: [PATCH 1/4] ares_cancel(): ensure cancellation of all requests

2013-03-21 Thread Alexander Klauer

On 03/21/2013 11:45 AM, Tommie Gannert wrote:

2013/3/21 Alexander Klauer :

An invocation of ares_cancel() walks through the request list, calling
the callbacks of all pending requests on a channel. Previously, if such
a callback added a new request to the channel, the request list might
not end up empty, causing an abort by assertion failure. The present
commit ensures that all such newly added requests are cancelled
immediately and make it never into request list. Thus, the crash is
avoided, and it is made certain that upon return of ares_cancel(), there
are no requests whatsoever on the channel.


There are two possible contracts I could see for this:

  1) ares_cancel() guarantees that all active requests when entering the
 function will be cancelled.
  2) ares_cancel() guarantees there are no active requests when the
 function exits.

The patch clearly implements (2). What are your thoughts on semantics here?
I've never had to use ares_cancel(), so I don't know if it currently states (2).


The ares_cancel() specification makes no explicit statement regarding 
callbacks registering new requests. It does use the past tense 
("requests made"), but without any indication of whether the point of 
reference is function entry or function exit, so any case for either 
possibility will remain weak at best.



Personally, I would find it somewhat surprising if I couldn't create
new requests
within an ares_cancel()-executed callback, and would prefer (1).


Funny, for me it's just the other way round.

The best solution in such a case would of course be to support both 
behaviours. One could simply pass some kind of flag to ares_cancel(), 
but that would break the current API.


How about this:

* Make ares_destroy() more user friendly and allow callbacks to register 
new requests on the channel being destroyed. Those requests would then 
immediately be finished with ARES_EDESTRUCTION.
* Let ares_cancel() walk through the list, but don't assert it's empty 
afterwards.


Then, to get behaviour (1), one would call ares_cancel(), and for 
behaviour (2), you would use ares_destroy(), followed by ares_init(). A 
little awkward perhaps, but the API would not be broken.


Thoughts?

Best regards,
Alexander


[PATCH 3/4] ares_destroy() documentation: no new requests

2013-03-21 Thread Alexander Klauer
Clarify that no new requests may be added to a resolver channel that is
currently being destroyed.
---
 ares_destroy.3 |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ares_destroy.3 b/ares_destroy.3
index 79171ec..3724df1 100644
--- a/ares_destroy.3
+++ b/ares_destroy.3
@@ -33,7 +33,8 @@ invokes the callbacks for each pending query on the channel, 
passing a
 status of
 .BR ARES_EDESTRUCTION .
 These calls give the callbacks a chance to clean up any state which
-might have been stored in their arguments.
+might have been stored in their arguments. A callback must not add new
+requests to a channel being destroyed.
 .SH SEE ALSO
 .BR ares_init (3),
 .BR ares_cancel (3)
-- 
1.7.9.5



[PATCH 2/4] Documentation: properly document ARES_ECANCELLED

2013-03-21 Thread Alexander Klauer
This commit clarifies the behaviour of ares_cancel() with respect to
callbacks and adds missing documentation of ARES_ECANCELLED to the man
pages of the affected functions.
---
 ares_cancel.3|4 +++-
 ares_gethostbyaddr.3 |3 +++
 ares_gethostbyname.3 |3 +++
 ares_getnameinfo.3   |3 +++
 ares_query.3 |3 +++
 ares_search.3|3 +++
 ares_send.3  |3 +++
 7 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/ares_cancel.3 b/ares_cancel.3
index ef10ec5..c34d8c5 100644
--- a/ares_cancel.3
+++ b/ares_cancel.3
@@ -28,7 +28,9 @@ name service channel identified by \fIchannel\fP.  
\fBares_cancel\fP invokes
 the callbacks for each pending query on the channel, passing a status of
 .BR ARES_ECANCELLED .
 These calls give the callbacks a chance to clean up any state which
-might have been stored in their arguments.
+might have been stored in their arguments. If a callback adds a new request
+to the channel, that request will be cancelled as well, so that upon return
+of \fBares_cancel\fP, no requests remain.
 .SH SEE ALSO
 .BR ares_init (3)
 .BR ares_destroy (3)
diff --git a/ares_gethostbyaddr.3 b/ares_gethostbyaddr.3
index f589868..7727307 100644
--- a/ares_gethostbyaddr.3
+++ b/ares_gethostbyaddr.3
@@ -70,6 +70,9 @@ was not found.
 .B ARES_ENOMEM
 Memory was exhausted.
 .TP 19
+.B ARES_ECANCELLED
+The query was cancelled.
+.TP 19
 .B ARES_EDESTRUCTION
 The name service channel
 .I channel
diff --git a/ares_gethostbyname.3 b/ares_gethostbyname.3
index a578a53..6b24ea4 100644
--- a/ares_gethostbyname.3
+++ b/ares_gethostbyname.3
@@ -74,6 +74,9 @@ was not found.
 .B ARES_ENOMEM
 Memory was exhausted.
 .TP 19
+.B ARES_ECANCELLED
+The query was cancelled.
+.TP 19
 .B ARES_EDESTRUCTION
 The name service channel
 .I channel
diff --git a/ares_getnameinfo.3 b/ares_getnameinfo.3
index 7e4990c..d227606 100644
--- a/ares_getnameinfo.3
+++ b/ares_getnameinfo.3
@@ -109,6 +109,9 @@ was not found.
 .B ARES_ENOMEM
 Memory was exhausted.
 .TP 19
+.B ARES_ECANCELLED
+The query was cancelled.
+.TP 19
 .B ARES_EDESTRUCTION
 The name service channel
 .I channel
diff --git a/ares_query.3 b/ares_query.3
index 0c5df84..733fbc9 100644
--- a/ares_query.3
+++ b/ares_query.3
@@ -118,6 +118,9 @@ No name servers could be contacted.
 .B ARES_ENOMEM
 Memory was exhausted.
 .TP 19
+.B ARES_ECANCELLED
+The query was cancelled.
+.TP 19
 .B ARES_EDESTRUCTION
 The name service channel
 .I channel
diff --git a/ares_search.3 b/ares_search.3
index 4184b00..2c85d20 100644
--- a/ares_search.3
+++ b/ares_search.3
@@ -119,6 +119,9 @@ No name servers could be contacted.
 .B ARES_ENOMEM
 Memory was exhausted.
 .TP 19
+.B ARES_ECANCELLED
+The query was cancelled.
+.TP 19
 .B ARES_EDESTRUCTION
 The name service channel
 .I channel
diff --git a/ares_send.3 b/ares_send.3
index 48d90ab..b89abfe 100644
--- a/ares_send.3
+++ b/ares_send.3
@@ -73,6 +73,9 @@ No name servers could be contacted.
 .B ARES_ENOMEM
 Memory was exhausted.
 .TP 19
+.B ARES_ECANCELLED
+The query was cancelled.
+.TP 19
 .B ARES_EDESTRUCTION
 The name service channel
 .I channel
-- 
1.7.9.5



[PATCH 1/4] ares_cancel(): ensure cancellation of all requests

2013-03-21 Thread Alexander Klauer
An invocation of ares_cancel() walks through the request list, calling
the callbacks of all pending requests on a channel. Previously, if such
a callback added a new request to the channel, the request list might
not end up empty, causing an abort by assertion failure. The present
commit ensures that all such newly added requests are cancelled
immediately and make it never into request list. Thus, the crash is
avoided, and it is made certain that upon return of ares_cancel(), there
are no requests whatsoever on the channel.
---
 ares.h   |1 +
 ares_cancel.c|6 ++
 ares_gethostbyaddr.c |6 ++
 ares_gethostbyname.c |6 ++
 ares_getnameinfo.c   |6 ++
 ares_private.h   |3 +++
 ares_query.c |6 ++
 ares_search.c|6 ++
 ares_send.c  |6 ++
 9 files changed, 46 insertions(+)

diff --git a/ares.h b/ares.h
index 9b3f376..8a9b995 100644
--- a/ares.h
+++ b/ares.h
@@ -140,6 +140,7 @@ extern "C" {
 #define ARES_FLAG_NOALIASES (1 << 6)
 #define ARES_FLAG_NOCHECKRESP   (1 << 7)
 #define ARES_FLAG_EDNS  (1 << 8)
+/* #define ARES_FLAG_CANCELLING(1 << 9) */ /* defined in ares_private.h */
 
 /* Option mask values */
 #define ARES_OPT_FLAGS  (1 << 0)
diff --git a/ares_cancel.c b/ares_cancel.c
index e5bb050..012fec1 100644
--- a/ares_cancel.c
+++ b/ares_cancel.c
@@ -30,6 +30,11 @@ void ares_cancel(ares_channel channel)
   struct list_node* list_node;
   int i;
 
+  if (channel->flags & ARES_FLAG_CANCELLING)
+  {
+return; /* Already being cancelled */
+  }
+  channel->flags |= ARES_FLAG_CANCELLING;
   list_head = &(channel->all_queries);
   for (list_node = list_head->next; list_node != list_head; )
   {
@@ -60,4 +65,5 @@ void ares_cancel(ares_channel channel)
 ares__close_sockets(channel, &channel->servers[i]);
 }
   }
+  channel->flags &= ~ARES_FLAG_CANCELLING;
 }
diff --git a/ares_gethostbyaddr.c b/ares_gethostbyaddr.c
index 85862e2..f77dafd 100644
--- a/ares_gethostbyaddr.c
+++ b/ares_gethostbyaddr.c
@@ -66,6 +66,12 @@ void ares_gethostbyaddr(ares_channel channel, const void 
*addr, int addrlen,
 {
   struct addr_query *aquery;
 
+  if (channel->flags & ARES_FLAG_CANCELLING)
+{
+  callback(arg, ARES_ECANCELLED, 0, NULL);
+  return;
+}
+
   if (family != AF_INET && family != AF_INET6)
 {
   callback(arg, ARES_ENOTIMP, 0, NULL);
diff --git a/ares_gethostbyname.c b/ares_gethostbyname.c
index 2b27b2e..12e452b 100644
--- a/ares_gethostbyname.c
+++ b/ares_gethostbyname.c
@@ -95,6 +95,12 @@ void ares_gethostbyname(ares_channel channel, const char 
*name, int family,
 return;
   }
 
+  if (channel->flags & ARES_FLAG_CANCELLING)
+{
+  callback(arg, ARES_ECANCELLED, 0, NULL);
+  return;
+}
+
   if (fake_hostent(name, family, callback, arg))
 return;
 
diff --git a/ares_getnameinfo.c b/ares_getnameinfo.c
index 5b9f638..26521be 100644
--- a/ares_getnameinfo.c
+++ b/ares_getnameinfo.c
@@ -88,6 +88,12 @@ void ares_getnameinfo(ares_channel channel, const struct 
sockaddr *sa,
   struct nameinfo_query *niquery;
   unsigned int port = 0;
 
+  if (channel->flags & ARES_FLAG_CANCELLING)
+{
+  callback(arg, ARES_ECANCELLED, 0, NULL, NULL);
+  return;
+}
+
   /* Validate socket address family and length */
   if ((sa->sa_family == AF_INET) &&
   (salen == sizeof(struct sockaddr_in)))
diff --git a/ares_private.h b/ares_private.h
index ab5be5a..14b9ac2 100644
--- a/ares_private.h
+++ b/ares_private.h
@@ -113,6 +113,9 @@
 #define EDNSFIXEDSZ11/* Size of EDNS header */
 /* EDNS defines section **/
 
+#define ARES_FLAG_CANCELLING (1 << 9) /* flag indicating all requests
+ being cancelled on a channel */
+
 struct ares_addr {
   int family;
   union {
diff --git a/ares_query.c b/ares_query.c
index 4bc9c25..2e8949a 100644
--- a/ares_query.c
+++ b/ares_query.c
@@ -115,6 +115,12 @@ void ares_query(ares_channel channel, const char *name, 
int dnsclass,
   unsigned char *qbuf;
   int qlen, rd, status;
 
+  if (channel->flags & ARES_FLAG_CANCELLING)
+{
+  callback(arg, ARES_ECANCELLED, 0, NULL, 0);
+  return;
+}
+
   /* Compose the query. */
   rd = !(channel->flags & ARES_FLAG_NORECURSE);
   status = ares_create_query(name, dnsclass, type, channel->next_id, rd, &qbuf,
diff --git a/ares_search.c b/ares_search.c
index ec07640..1d3e518 100644
--- a/ares_search.c
+++ b/ares_search.c
@@ -54,6 +54,12 @@ void ares_search(ares_channel channel, const char *name, int 
dnsclass,
   const char *p;
   int status, ndots;
 
+  if (channel->flags & ARES_FLAG_CANCELLING)
+{
+  callback(arg, ARES_ECANCELLED, 0, NULL, 0);
+  return;
+}
+
   /* If name only yields one domain to search, then we don't have
* to keep extra state, so just do an ares_query().
*/
diff --git a/ares_send.c b/ares_send.c
index 1a450b1..81a4049 100644
--- a/ares_send.c
+++ b/ares_send.c
@@ -3

[PATCH 4/4] .gitignore: ignore patch files

2013-03-21 Thread Alexander Klauer
This commit adds a line to .gitignore to the effect that patch files
generated by 'git format-patch' are excluded from the repository.
---
 .gitignore |1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 19a0eae..0b1087a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -82,3 +82,4 @@ ares_timeout.pdf
 ares_version.pdf
 c-ares-*.tar.gz.asc
 ares_parse_mx_reply.pdf
+/[0-9]*.patch
-- 
1.7.9.5



Cosmetic bugs in http://c-ares.haxx.se/ares_parse_a_reply.html and http://c-ares.haxx.se/ares_parse_aaaa_reply.html

2013-03-21 Thread Alexander Klauer

Hi,

in the HTML pages mentioned in the subject, there are spurious '\fB' 
sequences in the synopses. Possibly they have to be changed to '\fP' in 
the source man pages, but I don't know enough roff to say for sure.


Best regards,
Alexander

--
Dr. Alexander Klauer
Competence Centre for High Performance Computing

Fraunhofer-Institut für Techno-
und Wirtschaftsmathematik ITWM
Fraunhofer-Platz 1
67663 Kaiserslautern
Tel.: +49 631 31600-4335
Fax : +49 631 31600-5335
Email: alexander.kla...@itwm.fraunhofer.de


Re: [PATCH 2/3] library init: documentation update

2013-03-18 Thread Alexander Klauer

On 03/15/2013 08:26 PM, Yang Tse wrote:

On Fri, Mar 15, 2013, Alexander Klauer
 wrote:


This commit updates the documentation of ares_library_init() and
ares_library_cleanup() with regard to the new recursive behaviour.


It might be better to use "Its reference counted
initialize/deinitialize behavior," or something similar, instead of
"Its recursive behavior,"



OK, I've updated the commit.

Best regards,
Alexander


[PATCH 2/3] library init: documentation update

2013-03-18 Thread Alexander Klauer
This commit updates the documentation of ares_library_init() and
ares_library_cleanup() with regard to the newly introduced reference
counting of initializations and deinitializations.
---
 ares_library_cleanup.3 |   19 ---
 ares_library_init.3|   18 --
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/ares_library_cleanup.3 b/ares_library_cleanup.3
index 9fcf896..d60c378 100644
--- a/ares_library_cleanup.3
+++ b/ares_library_cleanup.3
@@ -31,11 +31,19 @@ The
 .B ares_library_cleanup
 function uninitializes the c-ares library, freeing all resources
 previously acquired by \fIares_library_init(3)\fP when the library
-was initialized.
+was initialized, provided there was only one single previous call to
+\fIares_library_init(3)\fP. If there was more than one previous call to
+\fIares_library_init(3)\fP, this function uninitializes the c-ares
+library only if it is the call matching the call to
+\fIares_library_init(3)\fP which initialized the library
+(usually the very first call to \fIares_library_init(3)\fP).
+Other calls to \fIares_library_cleanup(3)\fP have no effect other than
+decrementing an internal counter.
 .PP
 This function must be called when the program using c-ares will
 no longer need any c-ares function. Once the program has called
-\fIares_library_cleanup(3)\fP it shall not make any further call to any
+\fIares_library_cleanup(3)\fP sufficiently often such that the
+library is uninitialised, it shall not make any further call to any
 c-ares function.
 .PP
 This function does not cancel any pending c-ares lookups or requests
@@ -54,7 +62,12 @@ the DllMain function. Doing so will produce deadlocks and 
other problems.
 .SH AVAILABILITY
 This function was first introduced in c-ares version 1.7.0 along with the
 definition of preprocessor symbol \fICARES_HAVE_ARES_LIBRARY_CLEANUP\fP as an
-indication of the availability of this function.
+indication of the availability of this function. Reference counting in
+\fIares_library_init()\fP and \fIares_library_cleanup()\fP, which requires
+calls to the former function to match calls to the latter, is present since
+c-ares version 1.10.0.
+Earlier versions would deinitialize the library on the first call
+to \fIares_library_cleanup()\fP.
 .PP
 Since the introduction of this function, it is absolutely mandatory to call it
 for any Win32/64 program using c-ares.
diff --git a/ares_library_init.3 b/ares_library_init.3
index b3efc18..797476b 100644
--- a/ares_library_init.3
+++ b/ares_library_init.3
@@ -33,13 +33,15 @@ function performs initializations internally required by 
the c-ares
 library that must take place before any other function provided by
 c-ares can be used in a program.
 .PP
-This function must be called one time within the life of a program,
+This function must be called at least once within the life of a program,
 before the program actually executes any other c-ares library function.
-Initializations done by this function remain effective until a
-call to \fIares_library_cleanup(3)\fP is performed.
+Initializations done by this function remain effective until a number of
+calls to \fIares_library_cleanup(3)\fP equal to the number of calls to
+this function are performed.
 .PP
-Successive calls to this function do nothing, only the first call done
-when c-ares is in an uninitialized state is actually effective.
+Successive calls to this function do nothing further, only the first
+call done when c-ares is in an uninitialized state is actually
+effective.
 .PP
 The
 .I flags
@@ -77,7 +79,11 @@ non-zero error number will be returned to indicate the 
error. Except for
 .SH AVAILABILITY
 This function was first introduced in c-ares version 1.7.0 along with the
 definition of preprocessor symbol \fICARES_HAVE_ARES_LIBRARY_INIT\fP as an
-indication of the availability of this function.
+indication of the availability of this function. Its recursive behavior,
+which requires a matching number of calls to \fIares_library_cleanup()\fP
+in order to deinitialize the library, is present since c-ares version
+1.10.0. Earlier versions would deinitialize the library on the first call
+to \fIares_library_cleanup()\fP.
 .PP
 Since the introduction of this function it is absolutely mandatory to
 call it for any Win32/64 program using c-ares.
-- 
1.7.9.5



[PATCH 3/3] .gitignore: ignore patch files

2013-03-15 Thread Alexander Klauer
This commit adds a line to .gitignore to the effect that patch files
generated by 'git format-patch' are excluded from the repository.
---
 .gitignore |1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 19a0eae..0b1087a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -82,3 +82,4 @@ ares_timeout.pdf
 ares_version.pdf
 c-ares-*.tar.gz.asc
 ares_parse_mx_reply.pdf
+/[0-9]*.patch
-- 
1.7.9.5



[PATCH 2/3] library init: documentation update

2013-03-15 Thread Alexander Klauer
This commit updates the documentation of ares_library_init() and
ares_library_cleanup() with regard to the new recursive behaviour.
---
 ares_library_cleanup.3 |   18 +++---
 ares_library_init.3|   18 --
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/ares_library_cleanup.3 b/ares_library_cleanup.3
index 9fcf896..2071018 100644
--- a/ares_library_cleanup.3
+++ b/ares_library_cleanup.3
@@ -31,11 +31,19 @@ The
 .B ares_library_cleanup
 function uninitializes the c-ares library, freeing all resources
 previously acquired by \fIares_library_init(3)\fP when the library
-was initialized.
+was initialized, provided there was only one single previous call to
+\fIares_library_init(3)\fP. If there was more than one previous call to
+\fIares_library_init(3)\fP, this function uninitializes the c-ares
+library only if it is the call matching the call to
+\fIares_library_init(3)\fP which initialized the library
+(usually the very first call to \fIares_library_init(3)\fP).
+Other calls to \fIares_library_cleanup(3)\fP have no effect other than
+decrementing an internal counter.
 .PP
 This function must be called when the program using c-ares will
 no longer need any c-ares function. Once the program has called
-\fIares_library_cleanup(3)\fP it shall not make any further call to any
+\fIares_library_cleanup(3)\fP sufficiently often such that the
+library is uninitialised, it shall not make any further call to any
 c-ares function.
 .PP
 This function does not cancel any pending c-ares lookups or requests
@@ -54,7 +62,11 @@ the DllMain function. Doing so will produce deadlocks and 
other problems.
 .SH AVAILABILITY
 This function was first introduced in c-ares version 1.7.0 along with the
 definition of preprocessor symbol \fICARES_HAVE_ARES_LIBRARY_CLEANUP\fP as an
-indication of the availability of this function.
+indication of the availability of this function. Its recursive behavior,
+which requires calls to this functions to match calls to
+\fIares_library_cleanup(3)\fP, is present since c-ares version 1.10.0.
+Earlier versions would deinitialize the library on the first call
+to \fIares_library_cleanup()\fP.
 .PP
 Since the introduction of this function, it is absolutely mandatory to call it
 for any Win32/64 program using c-ares.
diff --git a/ares_library_init.3 b/ares_library_init.3
index b3efc18..797476b 100644
--- a/ares_library_init.3
+++ b/ares_library_init.3
@@ -33,13 +33,15 @@ function performs initializations internally required by 
the c-ares
 library that must take place before any other function provided by
 c-ares can be used in a program.
 .PP
-This function must be called one time within the life of a program,
+This function must be called at least once within the life of a program,
 before the program actually executes any other c-ares library function.
-Initializations done by this function remain effective until a
-call to \fIares_library_cleanup(3)\fP is performed.
+Initializations done by this function remain effective until a number of
+calls to \fIares_library_cleanup(3)\fP equal to the number of calls to
+this function are performed.
 .PP
-Successive calls to this function do nothing, only the first call done
-when c-ares is in an uninitialized state is actually effective.
+Successive calls to this function do nothing further, only the first
+call done when c-ares is in an uninitialized state is actually
+effective.
 .PP
 The
 .I flags
@@ -77,7 +79,11 @@ non-zero error number will be returned to indicate the 
error. Except for
 .SH AVAILABILITY
 This function was first introduced in c-ares version 1.7.0 along with the
 definition of preprocessor symbol \fICARES_HAVE_ARES_LIBRARY_INIT\fP as an
-indication of the availability of this function.
+indication of the availability of this function. Its recursive behavior,
+which requires a matching number of calls to \fIares_library_cleanup()\fP
+in order to deinitialize the library, is present since c-ares version
+1.10.0. Earlier versions would deinitialize the library on the first call
+to \fIares_library_cleanup()\fP.
 .PP
 Since the introduction of this function it is absolutely mandatory to
 call it for any Win32/64 program using c-ares.
-- 
1.7.9.5



[PATCH] library init: be recursive

2013-03-15 Thread Alexander Klauer
Previously, a single call to ares_library_cleanup() would deinitialise
the c-ares library, regardless of how many times ares_library_init() was
called. This behaviour may cause problems in programs linking two or
more libraries which, in turn, use c-ares. The present commit fixes this
problem, deinitializing the library only after a number of calls to
ares_library_cleanup() matching the number of calls to
ares_library_init().
---
 ares_library_init.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/ares_library_init.c b/ares_library_init.c
index f0137a1..770e7c2 100644
--- a/ares_library_init.c
+++ b/ares_library_init.c
@@ -101,7 +101,10 @@ int ares_library_init(int flags)
   int res;
 
   if (ares_initialized)
-return ARES_SUCCESS;
+{
+  ares_initialized++;
+  return ARES_SUCCESS;
+}
   ares_initialized++;
 
   if (flags & ARES_LIB_INIT_WIN32)
@@ -122,6 +125,8 @@ void ares_library_cleanup(void)
   if (!ares_initialized)
 return;
   ares_initialized--;
+  if (ares_initialized)
+return;
 
   if (ares_init_flags & ARES_LIB_INIT_WIN32)
 ares_win32_cleanup();
-- 
1.7.9.5