Subject: Re: [PATCH] ares_parse_txt_reply: add `record_start` field

Re: [PATCH] ares_parse_txt_reply: add `record_start` field

From: David Drysdale <drysdale_at_google.com>
Date: Wed, 3 Feb 2016 15:48:04 +0000

Still seems the name of the new function is inconsistent:
 - the code and manpage have: ares_parse_txt_reply_ext
 - the master ares.h header file has: ares_parse_txt_ext
 - the commit message has: ares_parse_txt_reply_ex

Have you run this code?

D.

On Mon, Feb 1, 2016 at 11:10 PM, Fedor Indutny <fedor_at_indutny.com> wrote:

> Thank you, Daniel.
>
> Hopefully, I have fixed all mentioned nits. I tried to do my best on the
> documentation, but please forgive me some grammar issues since I am not a
> native english speaker.
>
> If you have any other suggestions with regards to either code, docs, or
> both. I will be more than happy to put them into the patch.
>
> Thanks again,
> Fedor.
>
> On Mon, Feb 1, 2016 at 5:29 PM, Daniel Stenberg <daniel_at_haxx.se> wrote:
>
>> On Mon, 1 Feb 2016, Fedor Indutny wrote:
>>
>> I have made fixes to the patch from 2015-03-21, according to your
>>> comments.
>>>
>>> Please take a look.
>>>
>>
>> Two comments from me:
>>
>> 1. Please don't refer to source code file names in ares.h. That's a
>> public header file and users of c-ares will read that and have no idea
>> about the specific source file or what we're talking about. You could of
>> course still explain that the new struct is a superset of the old one.
>>
>> 2. Regarding the documentation/functionality. The new man page paragraph
>> says:
>>
>> "Continuations are linked using next pointer field in the structure"
>>
>> (I think it should say "*the* next pointer...")
>>
>> ... but it doesn't really explain how it works with any details. Should
>> the user then consider the 'txt' areas to be joined? I don't think we can
>> presume users will understand what a continuation is if we don't explain it!
>>
>> --
>>
>> / daniel.haxx.se
>>
>
>
Received on 2016-02-03