On Wed, 2009-12-02 at 15:07 +0100, Martin Nagy wrote:
> On Tue, 2009-12-01 at 19:33 +0100, Jakub Hrozek wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> >
> > c-ares 1.7.0 was released today and is already in rawhide. It is the
> > first released version that contains the code we contributed.
> >
> > What we have in HEAD right now for SRV and TXT parsing was based upon
> > the parsing functions that were in c-ares CVS at the time. During the
> > c-ares 1.7.0 release cycle there were some changes including renaming of
> > public structures and change in parameters in the parsing functions.
> > This means our code no longer compiles on rawhide. The attached patches
> > solve that, the code also works with older c-ares releases that are in
> > released versions of Fedora.
> >
> > I realize that we actually don't use the SRV and TXT parsing anywhere
> > yet -- so one option might be to remove the code altogether from 1.0.
> > But I think we're likely to add true service discovery post-1.0 and we'd
> > be adding the code back anyway..and keep rebasing the patches in the
> > meantime.
>
> I think we should just push these patches (after the review is done) and
> then disable them in the build & configure system like Stephen did for
> ELAPI (although FWIW, I think it would be nice to keep the tests
> running).
>
> > [PATCH 1/2] Change ares usage to be c-ares 1.7.0 compatible
> > * Rename structure accordingly to ares upstream
> > * Use new ares parsing functions in the wrappers
> > * fix tests for ares 1.7
>
> >From rewrite_talloc_srv_reply():
> + if (ptr->host == NULL) {
> + talloc_free(ptr);
> return ENOMEM;
> }
> +
> I believe this should be talloc_free(new_list), otherwise you might try
> to free a pointer pointing into middle of a talloc'ed memory. Also, is
> it wise to allocate an array for a linked list? I would recommend
> allocating each list item separately using mem_ctx and maybe provide a
> free function that would free all the items.
>
> + ptr = ptr->next;
> + old_list = old_list->next;
> This is also bad, since ptr->next is not initialized.
>
> Same goes for rewrite_talloc_txt_reply().
>
> > [PATCH 2/2] Import ares 1.7.0 helpers
> > Synchronizes the bundled helpers with the changes done during the c-ares
> > 1.7.0 development in order for the SRV and TXT parsing routines to work
> > also with pre-1.7.0 ares versions.
>
> Sorry, I didn't yet have time to review it, I will do it later (tomorrow
> probably).
Ok, patch 2/2:
in server/resolv/ares/ares_data.h:
+#ifndef HAVE_ARES_PARSE_TXT
+#include "resolv/ares/ares_parse_txt_reply.h"
+#endif /* HAVE_ARES_PARSE_TXT */
+
+#ifndef HAVE_ARES_PARSE_SRV
+#include "resolv/ares/ares_parse_srv_reply.h"
+#endif /* HAVE_ARES_PARSE_SRV */
You are using HAVE_ARES_PARSE_* macros but you removed them from the
configure checks.
I also checked the differences between files in server/ares/ and
original files from upstream and the changes seem to be reasonable.
Martin
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
