I'm having a real stellar day, diregard my last comment :) donald
On Tue, Jul 28, 2015 at 11:26 AM, Donald Sharp <[email protected]> wrote: > 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
