Michael -

There are two calls into OSPF_OPAQUE_TIMER_ON in ospf_opaque.c.  Lines 1805
and 2026.  Your patch fixes line 2026 but neglects 1805.

thanks!

donald

On Mon, Jul 27, 2015 at 3:14 PM, Michael Rossberg <
[email protected]> wrote:

> Hi,
>
> > This is a great idea, thanks for the implementation!
>
> I am happy if someone finds it useful.
>
> >  struct timeval
> > +msec2tv (int a)
> > +{
> > +  struct timeval ret;
> > +
> > +  ret.tv_sec = 0;
> > +  ret.tv_usec = a * 1000;
> > +
> > +  return tv_adjust (ret);
> > +}
> >
> >
> > I'm not a very big fan of returning local static information in
> msec2tv.  I would prefer that you pass in the struct timeval * that you
> want filled in.  Otherwise you are going to get some very weird behavior
> when someone down the line missappropriates the msec2tv and calls it twice
> and expects the output to stay consistent.  Additionally if someone ever
> adds another thread it will misbehave as well.
>
> Please note that the value is returned by copy and not by reference to a
> static or
> global variable. Hence, it should work perfectly in multithreaded
> environments or
> over multiple calls. It may not be the fastest, but struct timeval is
> small.
> >
> >    if (! list_isempty (ospf_opaque_type11_funclist)
> > @@ -1368,7 +1368,7 @@ ospf_opaque_lsa_originate_schedule (struct
> ospf_interface *oi, int *delay0)
> >        top->t_opaque_lsa_self =
> >          thread_add_timer (master, ospf_opaque_type11_lsa_originate,
> >                            top, delay);
> > -      delay += OSPF_MIN_LS_INTERVAL;
> > +      delay += top->min_ls_interval / 1000;
> >
> > delay is an integer, do you really mean to throw away the sub second
> accuracty here?  If I am reading the code right, I am able to setup a
> min_ls_interval of <0-5000>.  If I pass in 1500, I want 1.5 seconds, we are
> going to get 1 second.
>
> You are right - I have been lazy. Please find the attached patch that
> fixes this
> behavior.
> Is there anything else we should do here?
> Best regards
>
> Michael
>
>
>
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev

Reply via email to