slightly OT: error handling conventions

Discussion in 'C Programming' started by Matt Garman, Oct 20, 2003.

  1. Matt Garman

    Matt Garman Guest

    I was wondering what conventions people use for error handling in a
    function while avoiding memory leaks at the same time.

    I write a lot of functions that end up looking like the following:

    int my_func() {
    char *a, *b;
    int x;

    a = malloc(some_number);

    x = another_function(a);
    if (x == ERROR_CONDITION) {
    free(a);
    return ERROR;
    }

    x = func_that_mallocs(&b);
    if (x == ERROR_CONDITION) {
    free(a);
    free(b);
    return ERROR;
    }

    free(a);
    free(b);

    return NO_ERROR;
    }

    As you can see, I essentially have a lot of redundant code. Plus,
    this code could easily become a maintenance headache. Basically, I
    often do a malloc(), perform some calculations or other
    administrativia, maybe do some more malloc(), perform calculations,
    etc. At each step along the way, I do error checking (I'm trying to
    write robust code). If an error occurs, I need to exit the function,
    but I also need to free any malloc'ed memory.

    Is there a general strategy for dealing with this kind of scenario?
    It seems like a goto might be useful here. Are there any other "nice"
    conventions (readable, maintainable, etc)?

    Thanks,
    Matt

    --
    Please see the following for email info:
    http://raw-sewage.net/index.php?file=email
     
    Matt Garman, Oct 20, 2003
    #1
    1. Advertising

  2. Matt Garman

    Eric Sosman Guest

    Matt Garman wrote:
    >
    > I was wondering what conventions people use for error handling in a
    > function while avoiding memory leaks at the same time.
    >
    > I write a lot of functions that end up looking like the following:
    >
    > int my_func() {
    > char *a, *b;
    > int x;
    >
    > a = malloc(some_number);
    >
    > x = another_function(a);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > return ERROR;
    > }
    >
    > x = func_that_mallocs(&b);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > free(b);
    > return ERROR;
    > }
    >
    > free(a);
    > free(b);
    >
    > return NO_ERROR;
    > }
    >
    > As you can see, I essentially have a lot of redundant code. Plus,
    > this code could easily become a maintenance headache. Basically, I
    > often do a malloc(), perform some calculations or other
    > administrativia, maybe do some more malloc(), perform calculations,
    > etc. At each step along the way, I do error checking (I'm trying to
    > write robust code). If an error occurs, I need to exit the function,
    > but I also need to free any malloc'ed memory.
    >
    > Is there a general strategy for dealing with this kind of scenario?
    > It seems like a goto might be useful here. Are there any other "nice"
    > conventions (readable, maintainable, etc)?


    `goto' is a possibility. Nested `if' plus a variable to
    keep track of the return status is another:

    int my_func(void) {
    char *a, *b;
    int result = ERROR;
    a = malloc(some_number);
    if (a != NULL) { // a test you omitted
    x = another_function(a);
    if (x != ERROR_CONDITION) {
    x = func_that_mallocs(&b);
    if (x != ERROR_CONDITION) {
    do_something(); // another omission
    result = NO_ERROR;
    }
    free (b); // dubious, but mimics your code
    }
    free (a);
    }
    return result;
    }

    .... but this can become unwieldy. If there are a lot of things
    to test, `goto' may be a good deal clearer:

    int my_func(void) {
    char *a = NULL, *b = NULL;
    int result = ERROR;
    a = malloc(some_number);
    if (a == NULL) goto EXIT;
    x = another_function(a);
    if (x == ERROR_CONDITION) goto EXIT;
    x = func_that_mallocs(&b);
    if (x == ERROR_CONDITION) goto EXIT;
    do_something();
    result = NO_ERROR;
    EXIT:
    free (b);
    free (a);
    return result;
    }

    Note that the initializations of `a' and `b' allow you to have
    just a single exit-point instead of many.

    If a function has very many such conditions, consider whether
    it might make more sense to break it up into multiple smaller
    functions, each able to clean up after itself and "pass the buck"
    to the caller.

    And then, of course, there's always `abort()' ...

    --
     
    Eric Sosman, Oct 20, 2003
    #2
    1. Advertising

  3. Matt Garman

    Ian Woods Guest

    Matt Garman <> wrote in
    news::

    > I was wondering what conventions people use for error handling in a
    > function while avoiding memory leaks at the same time.
    >
    > I write a lot of functions that end up looking like the following:
    >
    > int my_func() {
    > char *a, *b;
    > int x;
    >
    > a = malloc(some_number);
    >
    > x = another_function(a);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > return ERROR;
    > }
    >
    > x = func_that_mallocs(&b);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > free(b);
    > return ERROR;
    > }
    >
    > free(a);
    > free(b);
    >
    > return NO_ERROR;
    > }
    >
    > As you can see, I essentially have a lot of redundant code. Plus,
    > this code could easily become a maintenance headache. Basically, I
    > often do a malloc(), perform some calculations or other
    > administrativia, maybe do some more malloc(), perform calculations,
    > etc. At each step along the way, I do error checking (I'm trying to
    > write robust code). If an error occurs, I need to exit the function,
    > but I also need to free any malloc'ed memory.
    >
    > Is there a general strategy for dealing with this kind of scenario?
    > It seems like a goto might be useful here. Are there any other "nice"
    > conventions (readable, maintainable, etc)?
    >
    > Thanks,
    > Matt
    >
    > --
    > Please see the following for email info:
    > http://raw-sewage.net/index.php?file=email


    For code where I have multiple resources to find, use, then release I
    often use the idea of a 'runlevel'. Basically, it's this:

    int do_stuff(char * iname, char * oname) {
    int rl=0; /* the 'runlevel' variable */
    int rv=ERROR; /* the error return value */
    FILE * inf;
    FILE * outf;
    void * mem;

    inf=fopen(iname,"r");
    if (!inf) goto on_err;
    else rl++; /* opened a resource, go up a level */

    outf=fopen(oname,"w");
    if (!outf) goto on_err;
    else rl++; /* opened a resource, go up a level */

    mem=malloc(WHATEVER_SIZE);
    if (!mem) goto on_err;
    else rl++; /* opened a resource, go up a level */

    /* do stuff with inf, outf and mem */

    rv=0; /* the success return value */

    on_err:
    switch(rl) {
    case 3: free(mem);
    case 2: fclose(outf);
    case 1: fclose(inf);
    case 0: /* nothing to clean up */
    }

    return rv;
    }

    There are plenty of ways this can be improved, but I find this particular
    method to be easy to code and easy to analyse. I wouldn't expect most
    people to just 'see' what was going on, so it's likely to be a pain for
    someone else to maintain - at least until they're familiar with the idea.

    Ian Woods
     
    Ian Woods, Oct 20, 2003
    #3
  4. On Mon, 20 Oct 2003 21:42:32 +0000, Matt Garman wrote:

    > I write a lot of functions that end up looking like the following:
    >
    > int my_func() {
    > char *a, *b;
    > int x;
    >
    > a = malloc(some_number);


    Check to see that malloc succeeded

    >
    > x = another_function(a);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > return ERROR;
    > }
    >
    > x = func_that_mallocs(&b);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > free(b);
    > return ERROR;
    > }
    >
    > free(a);
    > free(b);
    >
    > return NO_ERROR;
    > }
    >
    > As you can see, I essentially have a lot of redundant code.

    <snip>
    > Is there a general strategy for dealing with this kind of scenario?
    > It seems like a goto might be useful here. Are there any other "nice"
    > conventions (readable, maintainable, etc)?


    I think using goto is a reasonable (readable, maintainable, etc.)
    alternative for situations like this. However, I would make sure that
    func_that_mallocs() cleans up after itself if it fails instead of
    requiring its caller to do it.

    -Sheldon
     
    Sheldon Simms, Oct 20, 2003
    #4
  5. Matt Garman

    Jack Klein Guest

    On Mon, 20 Oct 2003 21:42:32 GMT, Matt Garman
    <> wrote in comp.lang.c:

    > I was wondering what conventions people use for error handling in a
    > function while avoiding memory leaks at the same time.
    >
    > I write a lot of functions that end up looking like the following:
    >
    > int my_func() {
    > char *a, *b;
    > int x;
    >
    > a = malloc(some_number);
    >
    > x = another_function(a);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > return ERROR;
    > }
    >
    > x = func_that_mallocs(&b);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > free(b);
    > return ERROR;
    > }
    >
    > free(a);
    > free(b);
    >
    > return NO_ERROR;
    > }
    >
    > As you can see, I essentially have a lot of redundant code. Plus,
    > this code could easily become a maintenance headache. Basically, I
    > often do a malloc(), perform some calculations or other
    > administrativia, maybe do some more malloc(), perform calculations,
    > etc. At each step along the way, I do error checking (I'm trying to
    > write robust code). If an error occurs, I need to exit the function,
    > but I also need to free any malloc'ed memory.
    >
    > Is there a general strategy for dealing with this kind of scenario?
    > It seems like a goto might be useful here. Are there any other "nice"
    > conventions (readable, maintainable, etc)?
    >
    > Thanks,
    > Matt


    I may get blasted for this, but I've always liked this one:

    int my_func()
    {
    char *a = NULL;
    char *b = NULL;
    FILE *fin = NULL;
    FILE *fout = NULL;
    int x = SUCCESS:

    do
    {
    a = malloc(...);
    b = malloc(...);
    if (!a || !b)
    {
    x = OUT_OF_MEMORY;
    break;
    }

    fin = fopen(...);
    fout = fopen(...);
    if (!fin || !fout)
    {
    x = FILE_OPEN_ERROR;
    break;
    }

    /* do the real work */
    } while (0);

    free(a);
    free(b);
    if (fin) fclose(fin);
    if (fout) fclose(fout);
    return x;
    }

    Of course the do { ... } while (0) with breaks is just a disguised
    goto, as indeed are all flow control structures, but inside a block
    like this you know they all go to the same place, you don't have to
    mentally check the goto labels to see if they are all the same.

    As an aside, I have often wished that fclose(NULL) was defined as a
    no-op, as is free(NULL), for the same reason of symmetry. Then you
    could always fclose() the pointer returned by fopen() whether
    successful or not, just as you can always free() the pointer returned
    by *alloc(), successful or not.

    --
    Jack Klein
    Home: http://JK-Technology.Com
    FAQs for
    comp.lang.c http://www.eskimo.com/~scs/C-faq/top.html
    comp.lang.c++ http://www.parashift.com/c -faq-lite/
    alt.comp.lang.learn.c-c++ ftp://snurse-l.org/pub/acllc-c /faq
     
    Jack Klein, Oct 21, 2003
    #5
  6. Matt Garman

    CBFalconer Guest

    Matt Garman wrote:
    >
    > I was wondering what conventions people use for error handling in
    > a function while avoiding memory leaks at the same time.
    >
    > I write a lot of functions that end up looking like the following:
    >
    > int my_func() {
    > char *a, *b;
    > int x;
    >
    > a = malloc(some_number);
    >
    > x = another_function(a);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > return ERROR;
    > }
    >
    > x = func_that_mallocs(&b);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > free(b);
    > return ERROR;
    > }
    >
    > free(a);
    > free(b);
    >
    > return NO_ERROR;
    > }
    >
    > As you can see, I essentially have a lot of redundant code. Plus,
    > this code could easily become a maintenance headache. Basically, I
    > often do a malloc(), perform some calculations or other
    > administrativia, maybe do some more malloc(), perform calculations,
    > etc. At each step along the way, I do error checking (I'm trying to
    > write robust code). If an error occurs, I need to exit the function,
    > but I also need to free any malloc'ed memory.
    >
    > Is there a general strategy for dealing with this kind of scenario?
    > It seems like a goto might be useful here. Are there any other "nice"
    > conventions (readable, maintainable, etc)?


    I would write that as:

    int my_func()
    {
    char *a, *b;
    int x;
    int err;

    a = b = NULL;
    if (!(a = malloc(some_number))) /* you omitted this test */
    err = NOMEM;
    else if (ERROR_CONDITION == (x = another_function(a)))
    err = ERROR;
    else if (ERROR_CONDITION == (x = func_that_mallocs(&b)))
    err = ERROR;
    else
    err = NO_ERROR;
    free(a);
    free(b)
    return err;
    }

    --
    Chuck F () ()
    Available for consulting/temporary embedded and systems.
    <http://cbfalconer.home.att.net> USE worldnet address!
     
    CBFalconer, Oct 21, 2003
    #6
  7. Ian Woods wrote:

    8> For code where I have multiple resources to find, use, then release I
    > often use the idea of a 'runlevel'. Basically, it's this:
    >
    > int do_stuff(char * iname, char * oname) {
    > int rl=0; /* the 'runlevel' variable */
    > int rv=ERROR; /* the error return value */
    > FILE * inf;
    > FILE * outf;
    > void * mem;
    >
    > inf=fopen(iname,"r");
    > if (!inf) goto on_err;
    > else rl++; /* opened a resource, go up a level */
    >

    <snip>
    >
    > on_err:
    > switch(rl) {
    > case 3: free(mem);
    > case 2: fclose(outf);
    > case 1: fclose(inf);
    > case 0: /* nothing to clean up */
    > }
    >
    > return rv;
    > }
    >
    > There are plenty of ways this can be improved, but I find this particular
    > method to be easy to code and easy to analyse. I wouldn't expect most
    > people to just 'see' what was going on, so it's likely to be a pain for
    > someone else to maintain - at least until they're familiar with the idea.


    I can certainly see what's going on, and I find your approach interesting,
    but I do have a question. I know you're not a fan of spaghetti programming.
    Here, though, there is little spaghetti to speak of, since you use goto to
    jump to the end of the function (straight to the error-handling, that is).
    This is the only use of goto I've ever seriously contemplated in C (because
    it's not exactly spaghetti - or rather, it's more like rigid, uncooked
    spaghetti, straight off the tree), but in fact I've always found other ways
    to do it.

    My question is this: Have you /considered/ other possibilities, especially
    w.r.t. maintenance issues, or are you content with the goto in this
    context? That is, have you explored this runlevel idea, /sans/ goto? If so,
    what were your findings?

    [Personal aside: Ian, I haven't forgotten, honest! I hope to get my email
    fixed up any month now.]

    --
    Richard Heathfield :
    "Usenet is a strange place." - Dennis M Ritchie, 29 July 1999.
    C FAQ: http://www.eskimo.com/~scs/C-faq/top.html
    K&R answers, C books, etc: http://users.powernet.co.uk/eton
     
    Richard Heathfield, Oct 21, 2003
    #7
  8. Matt Garman wrote:

    > I was wondering what conventions people use for error handling in a
    > function while avoiding memory leaks at the same time.
    >
    > I write a lot of functions that end up looking like the following:
    >
    > int my_func() {
    > char *a, *b;
    > int x;
    >
    > a = malloc(some_number);
    >
    > x = another_function(a);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > return ERROR;
    > }
    >
    > x = func_that_mallocs(&b);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > free(b);
    > return ERROR;
    > }
    >
    > free(a);
    > free(b);
    >
    > return NO_ERROR;
    > }
    >


    I've rewritten this in "my" style. See what you think.

    int my_func(void)
    {
    int rc = SUCCESS;
    char *a = malloc(some_number);
    char *b = NULL;

    if(a != NULL)
    {
    rc = another_function(a);
    if(SUCCESS == rc)
    {
    rc = func_that_mallocs(&b);
    if(SUCCESS == rc)
    {
    rc = DoStuff(a, b);
    free(b);
    }
    }
    free(a);
    }
    return rc;
    }

    --
    Richard Heathfield :
    "Usenet is a strange place." - Dennis M Ritchie, 29 July 1999.
    C FAQ: http://www.eskimo.com/~scs/C-faq/top.html
    K&R answers, C books, etc: http://users.powernet.co.uk/eton
     
    Richard Heathfield, Oct 21, 2003
    #8
  9. On Tue, 21 Oct 2003 06:05:24 +0000, Richard Heathfield wrote:

    > My question is this: Have you /considered/ other possibilities, especially
    > w.r.t. maintenance issues, or are you content with the goto in this
    > context? That is, have you explored this runlevel idea, /sans/ goto? If so,
    > what were your findings?


    Sorry, I'm not Ian, but I would like to ask a question of my own:

    What would be the advantage of eliminating goto in a situation
    like this?

    I once got involved in a large, er, "discussion" on the German C
    group about goto. No matter what situation was presented, the
    majority of those participating agreed that the use of goto was
    an abomination that should be avoided at all costs. Many of the
    same people also claimed to refuse to use break and continue.

    Their reasoning for this was hard for me to understand, since I
    lacked the cultural background, but apparently some influential
    professor had some ideas about program design which led most of
    them to require all programs to be designed by drawing boxes
    inside of boxes. Control was only permitted to flow into the
    top of a box or out of the bottom of a box.

    This was all nice, and I understood it to be a graphical approach
    to structured programming, but what I didn't understand was the
    attitude that *absolutely no* deviation from this schema was to
    be tolerated.

    -Sheldon
     
    Sheldon Simms, Oct 21, 2003
    #9
  10. In article <>,
    Sheldon Simms <> wrote:

    > I once got involved in a large, er, "discussion" on the German C
    > group about goto. No matter what situation was presented, the
    > majority of those participating agreed that the use of goto was
    > an abomination that should be avoided at all costs. Many of the
    > same people also claimed to refuse to use break and continue.
    >
    > Their reasoning for this was hard for me to understand, since I
    > lacked the cultural background, but apparently some influential
    > professor had some ideas about program design which led most of
    > them to require all programs to be designed by drawing boxes
    > inside of boxes. Control was only permitted to flow into the
    > top of a box or out of the bottom of a box.


    If anybody asks for advice whether to use goto or not, you should tell
    them not to use it. People who ask for advice shouldn't use goto.

    At some point they will not ask for advice anymore, and at some point
    they will either start thinking for themselves or they will not. If they
    think for themselves, they will use goto when it is appropriate and
    avoid it when it is inappropriate, and if they are smart they will know
    when it is appropriate. If they don't think for themselves, they will
    still insist that goto is always evil.

    Usual indication of "not thinking" is when you present an example that
    is more readable because it uses goto, and they insist that it is worse
    because it uses goto. So the conclusion that they draw is that using
    "goto" is bad because you use "goto".
     
    Christian Bau, Oct 21, 2003
    #10
  11. Sheldon Simms <> wrote in message news:<>...
    > On Tue, 21 Oct 2003 06:05:24 +0000, Richard Heathfield wrote:


    > > My question is this: Have you /considered/ other possibilities, especially
    > > w.r.t. maintenance issues, or are you content with the goto in this
    > > context? That is, have you explored this runlevel idea, /sans/ goto? If so,
    > > what were your findings?

    >
    > Sorry, I'm not Ian, but I would like to ask a question of my own:
    >
    > What would be the advantage of eliminating goto in a situation
    > like this?
    >
    > I once got involved in a large, er, "discussion" on the German C
    > group about goto. No matter what situation was presented, the
    > majority of those participating agreed that the use of goto was
    > an abomination that should be avoided at all costs. Many of the
    > same people also claimed to refuse to use break and continue.
    >
    > Their reasoning for this was hard for me to understand, since I
    > lacked the cultural background, but apparently some influential
    > professor had some ideas about program design which led most of
    > them to require all programs to be designed by drawing boxes
    > inside of boxes. Control was only permitted to flow into the
    > top of a box or out of the bottom of a box.
    >
    > This was all nice, and I understood it to be a graphical approach
    > to structured programming, but what I didn't understand was the
    > attitude that *absolutely no* deviation from this schema was to
    > be tolerated.


    "goto considered harmful"
    http://www.acm.org/classics/oct95/

    "structured programming with goto"
    http://pplab.snu.ac.kr/courses/PL2001/papers/p261-knuth.pdf

    then google "heathfield goto addict"

    (and Richard, don't let this goto your head...)


    --
    Nick Keighley

    "I've been on the wagon now for more than a decade.
    Not a single goto in all that time. I just don't
    need them any more. I don't even use break or continue
    now, except on social occasions of course.
    And I don't get carried away."
     
    Nick Keighley, Oct 21, 2003
    #11
  12. Matt Garman

    Dan Pop Guest

    In <> Matt Garman <> writes:

    >It seems like a goto might be useful here.


    A goto is useful any time it helps simplifying the program structure,
    especially reducing the number of indentation levels (but not at the cost
    of increasing the complexity of the program control structure, i.e.
    generating spaghetti code).

    Premature function termination (when cleanup is needed) is a typical case
    of goto usage.

    Dan
    --
    Dan Pop
    DESY Zeuthen, RZ group
    Email:
     
    Dan Pop, Oct 21, 2003
    #12
  13. Matt Garman

    David Rubin Guest

    Matt Garman wrote:
    >
    > I was wondering what conventions people use for error handling in a
    > function while avoiding memory leaks at the same time.
    >
    > I write a lot of functions that end up looking like the following:
    >
    > int my_func() {
    > char *a, *b;
    > int x;


    > a = malloc(some_number);


    > x = another_function(a);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > return ERROR;
    > }


    > x = func_that_mallocs(&b);
    > if (x == ERROR_CONDITION) {
    > free(a);
    > free(b);
    > return ERROR;
    > }


    > free(a);
    > free(b);


    > return NO_ERROR;
    > }


    Here's one more refactored implementation:

    int
    my_func(void)
    {
    char *a = 0;
    char *b = 0;
    int r = NO_ERROR;

    if((a=malloc(some_number)) == 0){
    return ERROR;
    }

    if(another_function(a) == ERROR_CONDITION
    || func_that_mallocs(&b) == ERROR_CONDITION){
    r = ERROR;
    }

    free(a);
    free(b);
    return r;
    }

    --
    Andre, a simple peasant, had only one thing on his mind as he crept
    along the East wall: 'Andre, creep... Andre, creep... Andre, creep.'
    -- unknown
     
    David Rubin, Oct 21, 2003
    #13
  14. Matt Garman

    Erik Domstad Guest

    Richard Heathfield <> wrote in message news:<bn2iqi$5jq$>...
    > I've rewritten this in "my" style. See what you think.
    >
    > int my_func(void)
    > {
    > int rc = SUCCESS;
    > char *a = malloc(some_number);
    > char *b = NULL;
    >
    > if(a != NULL)
    > {
    > rc = another_function(a);
    > if(SUCCESS == rc)
    > {
    > rc = func_that_mallocs(&b);
    > if(SUCCESS == rc)
    > {
    > rc = DoStuff(a, b);
    > free(b);
    > }
    > }
    > free(a);
    > }
    > return rc;
    > }


    I often write like this. It involves a few redundant tests of rc
    but it is not nested as deep. I even check rc the first time even though
    I know it's SUCCESS. This way it's easy to uncomment or move blocks of code.

    Perhaps not very elegant but I think it's quite readable and easy to maintain.

    int my_func(void)
    {
    int rc = SUCCESS;
    char *a = NULL;
    char *b = NULL;

    if (SUCCESS == rc) {
    a = malloc(some_number)
    if (NULL == a)
    rc = FAILURE;
    }

    if(SUCCESS == rc)
    rc = another_function(a);

    if(SUCCESS == rc)
    rc = func_that_mallocs(&b);

    if(SUCCESS == rc)
    rc = DoStuff(a, b);

    if (NULL != b)
    free(b);

    if (NULL != a)
    free(a);

    return rc;
    }
     
    Erik Domstad, Oct 21, 2003
    #14
  15. Matt Garman

    Ed Morton Guest

    Dan Pop wrote:

    > In <> Matt Garman <> writes:
    >
    >
    >>It seems like a goto might be useful here.

    >
    >
    > A goto is useful any time it helps simplifying the program structure,


    I agree in theory, but I've yet to see an example that wouldn't be more
    easily understandable using variables. e.g. elsethread a program
    structure like this was proposed (the only proposal I saw that used goto):

    void foo()
    int reached = 0;
    if (a() == 3) {
    goto cleanup;
    } else {
    reached++;
    }
    if (b() == 4) {
    goto cleanup;
    } else {
    reached++;
    }
    if (c() < 0) {
    goto cleanup;
    } else {
    reached++;
    }

    cleanup:
    switch (reached) {
    case 3: undoc();
    case 2: undob();
    case 1: undoa();
    }
    }

    Yes, we could all figure out what this does, especially if we were
    familiar with this style of coding, but why is a() returning 3
    apparently an error? Ditto for b() returning 4? Again, if the values
    tested for are obvious like NULL then again we can figure it out, but
    why write this type of code forcing future developers enhancing or
    debugging it to have to figure things out (i.e. synthesize the
    abstraction) when you can write something like this in the first place:

    void foo()
    enum { INIT, GOT_RSRCS, SENT_MSG, GOT_RSP } rslt = INIT;

    if (a() != 3) {
    rslt = GOT_RSRCS;
    }
    if (rslt == GOT_RSRCS) {
    if (b() != 4) {
    rslt = SENT_MSG;
    }
    }
    if (rslt == SENT_MSG) {
    if (c() >= 0) {
    rslt = GOT_RSP;
    }
    }

    cleanup:
    switch (rslt) {
    case GOT_RSP: undoc();
    case SENT_MSG: undob();
    case GOT_RSRCS: undoa();
    }
    }

    It's now immediately clear to next poor sap who has to work on this WHY
    we're going to cleanup. Bear in mind that same poor sap could have
    several hundred other lines of code to try to read and understand while
    trying to fix a bug that's going to cause the loss of a
    multi-million-dollar contract if not fixed by Friday!

    Among others, I've read the Knuth and Djikstra articles about use of
    gotos and I've yet to see an example of a reason to use them other than
    a very slight increase in execution efficiency (which is, of course,
    good enough reason in sme situations) but I'm open to persuasion.

    Ed.

    > especially reducing the number of indentation levels (but not at the cost
    > of increasing the complexity of the program control structure, i.e.
    > generating spaghetti code).
    >
    > Premature function termination (when cleanup is needed) is a typical case
    > of goto usage.
    >
    > Dan
     
    Ed Morton, Oct 21, 2003
    #15
  16. Matt Garman

    Dan Pop Guest

    In <bn3jge$> Ed Morton <> writes:


    >
    >Dan Pop wrote:
    >
    >> In <> Matt Garman <> writes:
    >>
    >>
    >>>It seems like a goto might be useful here.

    >>
    >>
    >> A goto is useful any time it helps simplifying the program structure,

    >
    >I agree in theory, but I've yet to see an example that wouldn't be more
    >easily understandable using variables. e.g. elsethread a program
    >structure like this was proposed (the only proposal I saw that used goto):
    >
    >void foo()


    Where is the brace?

    > int reached = 0;
    > if (a() == 3) {
    > goto cleanup;
    > } else {
    > reached++;
    > }
    > if (b() == 4) {
    > goto cleanup;
    > } else {
    > reached++;
    > }
    > if (c() < 0) {
    > goto cleanup;
    > } else {
    > reached++;
    > }
    >
    >cleanup:
    > switch (reached) {
    > case 3: undoc();
    > case 2: undob();
    > case 1: undoa();
    > }
    >}
    >
    >Yes, we could all figure out what this does, especially if we were
    >familiar with this style of coding, but why is a() returning 3
    >apparently an error? Ditto for b() returning 4? Again, if the values
    >tested for are obvious like NULL then again we can figure it out, but
    >why write this type of code forcing future developers enhancing or
    >debugging it to have to figure things out (i.e. synthesize the
    >abstraction) when you can write something like this in the first place:
    >
    >void foo()


    Ditto.

    > enum { INIT, GOT_RSRCS, SENT_MSG, GOT_RSP } rslt = INIT;
    >
    > if (a() != 3) {
    > rslt = GOT_RSRCS;
    > }
    > if (rslt == GOT_RSRCS) {
    > if (b() != 4) {
    > rslt = SENT_MSG;
    > }
    > }
    > if (rslt == SENT_MSG) {
    > if (c() >= 0) {
    > rslt = GOT_RSP;
    > }
    > }
    >
    >cleanup:
    > switch (rslt) {
    > case GOT_RSP: undoc();
    > case SENT_MSG: undob();
    > case GOT_RSRCS: undoa();
    > }
    >}


    In many cases, it is relatively easy to determine what needs to be
    undone without explicitly keeping track of it. This leads to the much
    simpler:

    void foo()
    {
    if (a() != OK) goto cleanup;
    if (b() != OK) goto cleanup;
    c();

    cleanup:
    if (donec) undoc();
    if (doneb) undob();
    if (donea) undoa();
    }

    Note that this is pseudocode, the done* things being expressions, rather
    than variables and the undo* things being cleanup actions, rather than
    actual function calls.

    And given the triviality of the example, it can be rewritten without goto
    and without nested if's:

    a() == OK && b() == OK && c() ;

    but this is not the point. I'd rather use the goto version, BTW ;-)

    The things that usually need cleanup in a standard C program are
    dynamically allocated buffers and open streams. If the corresponding
    pointers are initialised to NULL, no check whatsoever is needed for
    freeing the buffers, an unconditional free() call is doing the job,
    only FILE pointers need to be checked and fclose() called if they are
    non-null.

    Similar tricks can be done for other resources that need to be released
    by non-portable C programs, in order to simplify the overall code
    structure. If all the gotos in a function have the same destination,
    few people have a valid reason for complaining.

    Dan
    --
    Dan Pop
    DESY Zeuthen, RZ group
    Email:
     
    Dan Pop, Oct 21, 2003
    #16
  17. On Tue, 21 Oct 2003 04:08:42 -0700, Nick Keighley wrote:

    > Sheldon Simms <> wrote in message news:<>...
    >> On Tue, 21 Oct 2003 06:05:24 +0000, Richard Heathfield wrote:

    >
    >> What would be the advantage of eliminating goto in a situation
    >> like this?

    >
    > "goto considered harmful"
    > http://www.acm.org/classics/oct95/
    >
    > "structured programming with goto"
    > http://pplab.snu.ac.kr/courses/PL2001/papers/p261-knuth.pdf


    Yes yes, thanks. I've read them before.

    But Dijkstra isn't God, and "Go To Considered Harmful" isn't
    holy scripture. As for Knuth, he's *for* goto, as long as
    it's properly used.

    So once again, what would be the advantage of eliminating
    goto in the code under discussion (which I reinsert below)?

    > int do_stuff(char * iname, char * oname) {
    > int rl=0; /* the 'runlevel' variable */
    > int rv=ERROR; /* the error return value */
    > FILE * inf;
    > FILE * outf;
    > void * mem;
    >
    > inf=fopen(iname,"r");
    > if (!inf) goto on_err;
    > else rl++; /* opened a resource, go up a level */
    >
    > outf=fopen(oname,"w");
    > if (!outf) goto on_err;
    > else rl++; /* opened a resource, go up a level */
    >
    > mem=malloc(WHATEVER_SIZE);
    > if (!mem) goto on_err;
    > else rl++; /* opened a resource, go up a level */
    >
    > /* do stuff with inf, outf and mem */
    >
    > rv=0; /* the success return value */
    >
    > on_err:
    > switch(rl) {
    > case 3: free(mem);
    > case 2: fclose(outf);
    > case 1: fclose(inf);
    > case 0: /* nothing to clean up */
    > }
    >
    > return rv;
    > }
     
    Sheldon Simms, Oct 21, 2003
    #17
  18. On Tue, 21 Oct 2003 09:38:21 +0100, Christian Bau wrote:

    > Usual indication of "not thinking" is when you present an example that
    > is more readable because it uses goto, and they insist that it is worse
    > because it uses goto.


    What they will insist is that the version without goto is more
    readable. And the reason they will give for that is precisely that
    there is no goto. Since "more readable" is a subjective judgement,
    it's impossible to argue the point, so I don't try anymore.
     
    Sheldon Simms, Oct 21, 2003
    #18
  19. Matt Garman

    CBFalconer Guest

    Christian Bau wrote:
    >

    .... snip ...
    >
    > If anybody asks for advice whether to use goto or not, you should tell
    > them not to use it. People who ask for advice shouldn't use goto.


    Excellent. The last sentence is sig material.

    --
    Chuck F () ()
    Available for consulting/temporary embedded and systems.
    <http://cbfalconer.home.att.net> USE worldnet address!
     
    CBFalconer, Oct 21, 2003
    #19
  20. Matt Garman

    CBFalconer Guest

    Richard Heathfield wrote:
    >

    .... snip ...
    >
    > I've rewritten this in "my" style. See what you think.
    >
    > int my_func(void)
    > {
    > int rc = SUCCESS;
    > char *a = malloc(some_number);
    > char *b = NULL;
    >
    > if(a != NULL)
    > {
    > rc = another_function(a);
    > if(SUCCESS == rc)
    > {
    > rc = func_that_mallocs(&b);
    > if(SUCCESS == rc)


    You have a hidden assumption here, i.e. that failure does not
    malloc(&b).

    > {
    > rc = DoStuff(a, b);
    > free(b);
    > }
    > }
    > free(a);
    > }
    > return rc;
    > }


    --
    Chuck F () ()
    Available for consulting/temporary embedded and systems.
    <http://cbfalconer.home.att.net> USE worldnet address!
     
    CBFalconer, Oct 21, 2003
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Andrea Williams

    Coding Conventions for C#

    Andrea Williams, Feb 25, 2004, in forum: ASP .Net
    Replies:
    6
    Views:
    3,486
    Dennis
    Mar 5, 2004
  2. =B=
    Replies:
    4
    Views:
    9,339
  3. Floppy Jellopy

    Namespaces and Naming conventions

    Floppy Jellopy, Jul 20, 2005, in forum: ASP .Net
    Replies:
    4
    Views:
    541
    Kevin Spencer
    Jul 21, 2005
  4. cppaddict
    Replies:
    1
    Views:
    594
    Oliver Wong
    Jul 28, 2006
  5. Replies:
    4
    Views:
    1,543
Loading...

Share This Page