On Wed, Jul 13, 2016 at 10:16:22AM +0200, Pavel Březina wrote: > On 07/12/2016 03:50 PM, Michal Židek wrote: > > On 07/12/2016 01:36 PM, Michal Židek wrote: > > > On 07/12/2016 01:15 PM, Pavel Březina wrote: > > > > On 07/12/2016 12:34 PM, Michal Židek wrote: > > > > > state->ipa_ctx->dyndns_ctx->last_refresh = time(NULL); > > > > > > > > LGTM but maybe we should place the check before this line? > > > > > > Not sure... I only added checks for the line with strcmp > > > (which is where it segfaulted). If I moved > > > it up where you suggest, there would be the question why > > > I do not check for example ipa_ctx or ipa_ctx->dyndns_ctx. > > > > > > But if you prefer having it there I can do it, but will > > > also add a comment that the checks are relevant for the > > > strcmp line. > > > > > > Michal > > > > > > > Ok, I moved it as Pavel suggested and added a comment > > that should make it clear why the checks are there > > and also amended the commit message as Jakub suggested. > > > > Michal > > Ack. > > I asked for that since in my opinion it is more natural to set the last > refresh time only upon successful refresh. But I'm not that familiar with > dyndns to have strong opinion on what is better, thus it was a quastion.
* master: b5f61f8963300c9ba011436f234e9e10224aff6d * sssd-1-13: ae8c7c4010cb8fda478526771ef12d2bb8bfeffa (I hope it's OK I moved the ticket into 1.13.x) _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
