Yikes; how embarrassing. You are right on all three counts; I'll get a
fixed patch out ASAP.

On Oct 8, 2016 2:38 PM, "Barret Rhoden" <[email protected]> wrote:

> hi -
>
> couple potential issues:
>
> On 2016-10-07 at 16:44 Dan Cross <[email protected]> wrote:
> > If the lease time is 1, then we wouldn't wait; that's a bug.
> > Clean up an obnoxious conditional.
> >
> > Change-Id: I25ad3c5ac3510d56a0dc3d37b464ca002236875b
> > Signed-off-by: Dan Cross <[email protected]>
> > ---
> >  tools/apps/ipconfig/main.c | 19 +++++++------------
> >  1 file changed, 7 insertions(+), 12 deletions(-)
> >
> > diff --git a/tools/apps/ipconfig/main.c b/tools/apps/ipconfig/main.c
> > index db353bd..a19197b 100644
> > --- a/tools/apps/ipconfig/main.c
> > +++ b/tools/apps/ipconfig/main.c
> > @@ -952,20 +952,17 @@ static void *dhcpwatchthr(void *arg)
> >       uint32_t t;
> >       int needconfig = (arg == NULL);
> >
> > -     // procsetname("dhcpwatch");
> >       /* keep trying to renew the lease */
> >       for (;;) {
> > +             secs = conf.lease / 2;
> >               if (conf.lease == 0)
> >                       secs = 5;
> > -             else
> > -                     secs = conf.lease >> 1;
>
> I think if conf.lease was 1 in the new code, then secs still = 0 after
> this.  The if check only checks if conf.lease was 0, but doesn't adjust
> sec otherwise.  If that's not OK with the overall code, then that bug
> is still here.
>
> >
> >               /* avoid overflows */
> >               for (s = secs; s > 0; s -= t) {
> > -                     if (s > Maxsleep)
> > +                     t = s;
> > +                     if (t > Maxsleep)
> >                               t = Maxsleep;
> > -                     else
> > -                             t = s;
> >                       usleep(t * 1000 * 1000);
> >               }
> >
> > @@ -1279,9 +1276,8 @@ void dhcprecv(void)
> >               /* get plan9-specific options */
> >               n = optgetvec(bp->optdata, OBvendorinfo, vopts,
> sizeof(vopts) - 1);
> >               if (n > 0 && parseoptions(vopts, n) == 0) {
> > -                     if (validip(conf.fs) && Oflag)
> > -                             n = 1;
> > -                     else {
> > +                     n = 1;
> > +                     if (!validip(conf.fs) && Oflag) {
>
> this looks a little suspicious, since you just did the ! for validip,
> but not for the other part of the && in the if check.  no need to change
> it if you still think it's ok.  i'm not particularly familiar with this
> code.
>
> >                               n = optgetp9addrs(vopts, OP9fs, conf.fs,
> 2);
> >                               if (n == 0)
> >                                       n = optgetaddrs(vopts, OP9fsv4,
> conf.fs, 2);
> > @@ -1289,9 +1285,8 @@ void dhcprecv(void)
> >                       for (i = 0; i < n; i++)
> >                               DEBUG("fs=%R ", conf.fs + i * IPaddrlen);
> >
> > -                     if (validip(conf.auth) && Oflag)
> > -                             n = 1;
> > -                     else {
> > +                     n = 1;
> > +                     if (!validip(conf.auth) && Oflag) {
>
> same here.
>
> >                               n = optgetp9addrs(vopts, OP9auth,
> conf.auth, 2);
> >                               if (n == 0)
> >                                       n = optgetaddrs(vopts, OP9authv4,
> conf.auth, 2);
>
> barret
>
> --
> You received this message because you are subscribed to the Google Groups
> "Akaros" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To post to this group, send email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Akaros" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to