change to break vs return
Matthias Dieter Wallnöfer
mdw at samba.org
Tue Nov 30 00:42:12 MST 2010
Hi Tridge,
tridge at 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
More information about the samba-technical
mailing list