[Asterisk-code-review] DNS: Create a system-level DNS resolver (asterisk[master])

[Ashley Sanders]

Ashley Sanders has posted comments on this change.

Change subject: DNS: Create a system-level DNS resolver
......................................................................


Patch Set 3:

(5 comments)

https://gerrit.asterisk.org/#/c/749/3/main/dns.c
File main/dns.c:

Line 51: /*! \brief Return code upon failure. */
       : #define DNS_SEARCH_FAILURE -1
       : 
       : /*! \brief Return code upon no records found. */
       : #define DNS_SEARCH_NO_RECORDS 0
       : 
       : /*! \brief Return code upon success. */
       : #define DNS_SEARCH_SUCCESS 1
> Okay. I can do that for the next revision.
Done.


Line 233: 	dns_response += frame_size;
> You're right. I saw this earlier and made a mental note to go back and upda
Done.


Line 470: 		/* Try updating the cursors for the current record */
        : 		if ((pos = dns_update_frame(answer, pos, res + sizeof(struct dn_answer)))  < 0) {
        : 			return DNS_SEARCH_FAILURE;
        : 		}
        : 
        : 		/* Cast the current value for the answer pointer as a dn_answer struct */
        : 		ans = (struct dn_answer *)answer;
> Ahh, you're right. I missed something. This, according to lines 378-386 was
Done.

I also added a space between the variable and the cast, to match the changes made to dns_system_resolver.c.


Line 481: 			if (record_handler) {
        : 				record_handler(context, answer, ntohs(ans->size), ans->ttl);
        : 				ret = DNS_SEARCH_SUCCESS;
        : 			}
> Oh, one more thought regarding point #1: the original code was not failing 
Done.


https://gerrit.asterisk.org/#/c/749/3/main/dns_system_resolver.c
File main/dns_system_resolver.c:

Line 185: 	int res = ast_taskprocessor_push(dns_system_resolver_tp,
        : 	                                 dns_system_resolver_process_query,
        : 	                                 ao2_bump(query));
        : 
        : 	/* The query processing handler was not added to the task processor */
        : 	if (res == DNS_SYSTEM_RESOLVER_FAILURE) {
> Checking res against DNS_SYSTEM_RESOLVER_FAILURE here seems odd considering
Done.


-- 
To view, visit https://gerrit.asterisk.org/749
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b36ea17b889a98df4f8d80d50bb7ee175afa077
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-HasComments: Yes