Hi Fedor,
Sorry, I'd forgotten about your patch when I looked at the similar patch
from
Brady recently. I guess if two different developers want essentially the
same
function (and are keen enough to send patches) we should pay a bit more
attention!
Personally, I'd lean towards not including the age / reserved extension
mechanism. None of the other structures that describe particular resource
records have it, and the first/rest marker is just an accidental omission
from
the TXT structure, so just having a new, complete, structure seems sensible.
(For the specific addition of a TTL field that Jakub mentions, I think it
would be
better to come up with a mechanism that works for all of the RR types.)
However, I'm happy to defer to Daniel's opinion, so I'd suggest leaving
as-is for now.
Other comments:
- Name of new API function doesn't match in header and code -- have you
used this patch in practice?
- No man-page update
- (Technically, I think the type-punning of the two structure types isn't
covered
by the common initial sequence rules (because the next pointers have
different types), but I would be amazed if that ever caused a problem in
practice.)
- Minor tweaks:
- ares_data.c:163: I like to add a /* FALLTHROUGH */ comment for
deliberate
case fall through (I have a vague memory that it shuts up some lint
warning
too).
- ares_data.h:22: add "introduced in 1.11.0" to the comment
- ares_parse_txt_reply.c:156: add brackets: record_start = (strptr ==
aptr);
Regards,
David
On Tue, Jan 26, 2016 at 1:57 AM, Fedor Indutny <fedor_at_indutny.com> wrote:
> Kindly reminding you about this.
>
> On Wed, May 27, 2015 at 3:24 PM, Fedor Indutny <fedor_at_indutny.com> wrote:
>
>> Ping? ;)
>>
>> On Mon, Apr 6, 2015 at 11:41 PM, Fedor Indutny <fedor_at_indutny.com> wrote:
>>
>>> Ping ;)
>>>
>>> On Sat, Mar 21, 2015 at 2:38 AM, Fedor Indutny <fedor_at_indutny.com>
>>> wrote:
>>>
>>>> Hello guys!
>>>>
>>>> Long time again :) Sorry, I was busy with various other stuff.
>>>>
>>>> Here is a patch with (hopefully) all suggested changes.
>>>>
>>>> Please take a look!
>>>>
>>>> Thank you,
>>>> Fedor.
>>>>
>>>> On Thu, Dec 11, 2014 at 5:05 AM, Jakub Hrozek <jhrozek_at_redhat.com>
>>>> wrote:
>>>>
>>>>> On Thu, Dec 11, 2014 at 09:54:47AM +0100, Daniel Stenberg wrote:
>>>>> > On Thu, 11 Dec 2014, Jakub Hrozek wrote:
>>>>> >
>>>>> > >>You want me to add a couple of `void*`?
>>>>> > >
>>>>> > >I was thinking:
>>>>> > > uint8_t reserved[16];
>>>>> >
>>>>> > Another way, which is one I've used elsewhere, is this:
>>>>> >
>>>>> > struct larger_in_future {
>>>>> > int age; /* generation of struct */
>>>>> > void *member; /* always present */
>>>>> > }
>>>>> >
>>>>> > And c-ares would always set age to 0 in the first generation. A
>>>>> future
>>>>> > extension of the struct would bump the age to 1 and extend the
>>>>> struct:
>>>>> >
>>>>> > struct larger_in_future {
>>>>> > int age; /* generation of struct */
>>>>> > void *member; /* always present */
>>>>> >
>>>>> > /* only present if 'age' is >= 1 */
>>>>> > void *later;
>>>>> > }
>>>>> >
>>>>> > It means a program has to check age before accessing struct fields
>>>>> below
>>>>> > 'member' which is a bit of an annoyance, but possibly doable as the
>>>>> bumping
>>>>> > shouldn't happen terrible often?
>>>>>
>>>>> I don't have a problem with that personally. It's a neat trick I
>>>>> haven't
>>>>> used before.
>>>>>
>>>>
>>>>
>>>
>>
>
Received on 2016-01-28