Handle separate response

[codable]

This PR aims at fixing #83. The theory is when receiving empty-message, it inserts the request into send queue and update the retransmit_cnt to be bigger than COAP_DEFAULT_MAX_RETRANSMIT, thus it will be dropped after the timer timeout, and the timeout is set be as if it is the last try of retransmission. When the actually response is received, the special request is removed immediately from the queue.

[codable]

any comments?

[obgm]

I haven't yet time to look into the patch, sorry. But will do soon.

[obgm]

This is a nice trick to handle the separate response issue, thanks for providing the patch. Can you please resubmit with the requested changes made and a commit message that follows the contribution guidelines? Thanks!

[obgm]

I presume this should be q instead of context->sendqueue, no?

[obgm]

(see previous comment)

[obgm]

Maybe we can encapsulate the condition ...retransmit_cnt > COAP_DEFAULT_MAX_RETRANSMIT in some way so that the semantics are easier to understand, e.g.:

static inline int
is_separate(const coap_queue_t *node) {
   return node->retransmit_cnt > COAP_DEFAULT_MAX_RETRANSMIT;
}

and then:

...coap_address_equals(....) && token_match(....) && is_separate(context->sendqueue)...

[obgm]

It took me some time to understand why this will actually work---can you include a comment that explains why in this case, the sendqueue may contain the sent request despite sent being NULL?

[codable]

Thanks for pointing out those mistakes and suggestions.

[codable]

any more comments? @obgm

[obgm]

Looks good at a first glance. I will do a quick test within the next days and merge on success,

[obgm]

coap_remove_separate_from_queue() removes a node from the send queue without deleting it afterwards. This results in a memory leak whenever a separate response is received before the maximum retransmission timeout is reached. You can check this e.g. with valgrind:

1. Start the example server: coap-server
2. From a different terminal, send a query for the async resource:
   valgrind -v coap-client -v9 -B 200 coap://[::1]/async?15
3. (Optional:) Compare with very late response:
   valgrind -v coap-client -v9 -B 200 coap://[::1] /async?150

An optimal solution to this should also avoid doubling most of the code in coap_cancel_all_messages().

[obgm]

Thanks for addressing the previous comments. After a few tests, I found a serious memory leak that needs to be fixed before this can be merged.

[codable]

@obgm I updated the code to free the memory by not removing the separate request from the send queue before calling the actually handler and free them after calling handler. But I'm sorry, I cannot build with examples to verify memory leak as you suggested. The error is

a2x --doctype manpage --format manpage coap-client.txt
a2x: ERROR: "xmllint" --nonet --noout --valid "libcoap/examples/coap-client.xml" returned non-zero exit status 4

[obgm]

Ugh, yes, saw that xmllint error before. IIRC, it has to do with a missing package that for some reason is not listed in the dependencies of a2x (some missing DTD or so). A quick and dirty workaround would be to pass --disable-documentation to configure.

[codable]

I tested and it seems no memory leak found now.

[codable]

any more comments?