Re: continuing continue - was Re: [patch] continue statement

[Rici Lake <lua@...>]

On 24-Sep-05, at 2:33 PM, Glenn Maynard wrote:

> A nice and
> clean way of doing it, though (in C) is:
>
> 	void *a = NULL, *b = NULL, *c = NULL, *d = NULL;
> 	a = foo();
> 	if(!a) goto error;
>
> 	b = foo();
> 	if(!c) goto error;
>
> 	c = foo();
> 	...
>
> 	if(!e) goto error;
> 	do work;
> 	return;
>
> 	error:
> 	if(a != NULL) destroy(a);
> 	if(b != NULL) destroy(b);
> 	if(c != NULL) destroy(c);
> 	...
>
> I prefer C++, where I can let dtors do the work.  In Lua, I'd expect 
> the
> GC to do all this, but if you need immediate, predictable cleanup 
> that's
> sometimes not good enough.

I would have thought that you should destroy a, b, c (and maybe d) even 
if there were no error :) You could write this easily in Lua:

function dosomething()
  local a, b, c, d
  local function cleanup(...)
    if a then destroy(a) end
    if b then destroy(b) end
    if c then destroy(c) end
    if d then destroy(d) end
    return ...
  end

  a = foo()
  if not a then return cleanup() end
  b = foo()
  if not b then return cleanup() end
  c = foo()
  if not c then return cleanup() end
  d = foo()
  if not d then return cleanup() end
  -- do work
  return cleanup(result)
end

In any event, since the above does not appear to be in a loop, a 
"continue" statement isn't going to help (although "try...finally" 
would reduce verbiage quite a bit.)

Not that I have any objection to "continue" -- I just don't find 
personally find it much of a loss. A rather unscientific survey I just 
did seems to indicate that it is not enormously used  in C, either: the 
following is from the /usr/src directory of a FreeBSD installation:

rlake@freeb:/usr/src$ find . -name "*.c" -type f | xargs cat | wc
 6022172 21106454 163772234
rlake@freeb:/usr/src$ find . -name "*.c" -type f | xargs grep -w 
"break;" | wc
   87921  196286 3682304
rlake@freeb:/usr/src$ find . -name "*.c" -type f | xargs grep -w 
"continue;" | wc
   12774   28538  552118
rlake@freeb:/usr/src$ find . -name "*.c" -type f | wc
   10870   10870  337904
rlake@freeb:/usr/src$ find . -name "*.c" -type f | xargs grep -l -w 
"break;" | wc
    5584    5584  166015
rlake@freeb:/usr/src$ find . -name "*.c" -type f | xargs grep -l -w 
"continue;" | wc
    2632    2632   75427


In other words, out of about six million lines of C (including 
comments), "break" statements account for about 1.5% and "continue" 
statements account for about 0.2%; of the almost 11,000 files, slightly 
more than half have at least one break statement, but slightly less 
than a quarter have a continue statement (and of those, more than half 
have only one or two). Out of curiosity, I also checked:

rlake@freeb:/usr/src$ find . -name "*.c" -type f | xargs grep -l -w 
"goto" | wc
    2958    2958   87880
rlake@freeb:/usr/src$ find . -name "*.c" -type f | xargs grep -w "goto" 
| wc
   27381   95725 1368591


(Doing a random check of continue statements in a few of the files 
which use a lot of them, I ran into the following gem:

        LIST_FOREACH(sp, &sptree[dir], chain) {
                if (sp->state == IPSEC_SPSTATE_DEAD)
                        continue;
                if (sp->spidx) {
                        if (!spidx)
                                continue;

                        KEYDEBUG(KEYDEBUG_IPSEC_DATA,
                                printf("*** in SPD\n");
                                kdebug_secpolicyindex(sp->spidx));

                        if (key_cmpspidx_withmask(sp->spidx, spidx))
                                goto found;
                }
        }

        splx(s);
        return NULL;

found:

------- which I think is more efficiently and more clearly written:

       if (spidx) {
               LIST_FOREACH(sp, &sptree[dir], chain) {
                       if (sp->state != IPSEC_SPSTATE_DEAD && 
sp->spidx) {
                              KEYDEBUG(KEYDEBUG_IPSEC_DATA,
                                      printf("*** in SPD\n");
                                      kdebug_secpolicyindex(sp->spidx));

                              if (key_cmpspidx_withmask(sp->spidx, 
spidx))
                                      goto found;
                       }
                }
        }

        splx(s);
        return NULL;

found:

----- although I'm surprised the goto police haven't caught up with it 
anyway :)