[prev in list] [next in list] [prev in thread] [next in thread]
List: samba-technical
Subject: Re: change to break vs return
From: Matthias_Dieter_Wallnöfer <mdw () samba ! org>
Date: 2010-11-30 7:42:12
Message-ID: 4CF4AAD4.5070906 () samba ! org
[Download RAW message or body]
Hi Tridge,
tridge@samba.org wrote:
> Hi Matthias,
>
> Your patch 54a5c398aaa1d53b5892c9b0aa5539ae7da651da creates an
> infinite loop. Please revert.
>
> > diff --git a/source4/torture/basic/base.c b/source4/torture/basic/base.c
> > index 9953573..0e168ed 100644
> > --- a/source4/torture/basic/base.c
> > +++ b/source4/torture/basic/base.c
> > @@ -1340,7 +1340,7 @@ static bool run_iometer(struct torture_context *tctx,
> > for (i=0; i<num_reads; i++) {
> > ssize_t res;
> > if (ops++> torture_numops) {
> > - return true;
> > + break;
>
> this break only breaks out of the for loop, not for surrounding while
> loop, so your change means that this code now never returns, whereas
> it used to return on a loop limit.
>
Sorry, I've overseen this and "make test" seems not to have run it.
> Can I suggest that you walk through your code changes with gdb before
> pushing? Also, a number of us have suggested that you ask for review
> before you push changes. Did you get anyone to review this?
>
Yes, this was shortly reviewed by Jelmer. But the problem is that you
don't immediately notice the nested loops - I should have run the test
myself before commiting.
Cheers,
Matthias
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic