On 08/09/2016 05:41 AM, Jakub Hrozek wrote:
On Fri, Aug 05, 2016 at 12:09:27PM -0400, Justin Stephenson wrote:
> Hi Lukas,
>
> I sent a response on July 6th but perhaps there was an issue with the
> mailing list or some reason it did not go through.
Yes, we had issues with the mailing list back then (it was a Fedora
mailman bug that was fixed in the meantime)
> Updated patch attached.
>
> I moved the resolv_is_address() function declaration into the
> async_resolv.h file(so that it could be included in the
> cmocka/test_resolv_fake.c test but I am not sure if this is the
> correct approach). I also made the assumption of including my tests
> in the already existing test_resolv_fake.c file instead of a
> different file.
>
> Also, I wasn't sure whether to use SSSDBG_MINOR_FAILURE or
> SSSDBG_CONF_SETTINGS debug log level.
I would say SSSDBG_IMPORTANT_INFO, we want to be loud here.
> I am sure there are some corrections to make so I appreciate any
> feedback.
I haven't tested the patch yet, just read the diff, but the code looks
OK to me. I would just suggest to split the patch into two, one that
makes the resolv function public and adds the test and the other that
uses the function in the providers.
I split the patch into 2 separate patches as
requested, see attached.
Testing with ad_server = <ad-ip-address> was successful with the debug
message being printed in the domain log.
Kind regards,
Justin Stephenson
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org