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. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
