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