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)