On Thu, Jan 28, 2016 at 11:42 PM, bdoetsch <bdoet...@ameritech.net> wrote:
> > @Fedor -- I think you and I are trying to get the same result, a TXT > parsing routine that clearly delineates between TXT chunk components. > Hopefully this submission can help us both. > Hi Brady, By the way, did you take a look at Fedor's suggested patches, particularly the one from 2015-03-21? Would that patch cover the functionality you need? > Daniel, David, et al., > > I've taken a second crack at a patch for this issue, taking into account > the excellent advice from David to include any new structs explicitly in > the headers, and give new structs their own parse routines, as opposed to > casting between common initial sequences, which is what I had done the > first time in an attempt to be minimal. (BTW, I believe casting between common initial sequences is legal if they're all in the same union, so I think that Fedor's patch is OK in that respect.) > Any feedback on this new code is welcome, I'd love more eyes on it to > make sure I haven't made mistakes, and I'm happy to make modifications if > need be. Because I've included associated tests, this patch is against > David's git repo (https://github.com/daviddrysdale/c-ares.git), commit > "2dd71de," because that is the fork that has the unit tests, but I believe > is current with the main project. > > I use a new "ares_txt_cmps_reply" struct which deprecates the > "ares_txt_reply" struct. It is parsed by its own function > "ares_parse_txt_cmps_reply()" which is a modified implementation of > "ares_parse_txt_reply()". The patch adds new file > "ares_parse_txt_cmps_reply.c" modifies existing files "ares.h," > "ares_data.[c|h]" is documented in new man page > "ares_parse_txt_cmps_reply.3," and is tested in new test file > "test/ares-test-parse-txt-cmps.cc". Looking back over the mailing list, I > saw that Jakub Hrozek requested adding TTL into the new struct if ever it > was implemented, so I've done that as well. Yeah, I don't actually agree with Jakub on that, as it's inconsistent with all of the other parsed RR structures -- I think it would be better to come up with another way of getting the TTL information out that applies to all RR types, not just TXT records. > Here's how it looks presently: > > struct ares_txt_cmps_reply { > struct ares_txt_cmps_reply *next; > unsigned char **txt_cmps; /* array of null terminated strings > */ > unsigned int *txt_cmps_lengths; /* length of each string in > the array, don't rely on '\0' */ > unsigned int txt_cmps_count; /* number of strings/lengths in > the array(s) */ > unsigned short dnstype; > unsigned short dnsclass; > I've not looked at the patch yet, but I'd wonder whether these two fields are necessary -- can they ever be anything other than IN(1) / TXT(16) ? > unsigned int ttl; > }; > > > To see the full submission, clone David's repo at the github url above, > checkout commit 2dd71de, perform a git-apply on that revision using the > attached patch. Let me know how you feel about the direction. I'm not > married to it, so feel free to speak your mind... > > My continued thanks, > > Brady > > > > > >
