Newbie needing some help with segmentation fault

Discussion in 'C Programming' started by Hendrik Maryns, Dec 4, 2007.

  1. Hi all,

    I received a tiny program from a colleague. I want to use it in my Java
    program. I invoke it with Java’s ProcessBuilder. That not being very
    relevant, the problem is the following: it causes a segfault. Debugging
    tells me the following line is the culprit:

    while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol) == 4)

    This is the stack:

    Thread [0] (Suspended: Signal 'SIGSEGV' received. Description:
    Segmentation fault.)
    4 _IO_vfscanf() 0x00002ae4d5a0565f
    3 fscanf() 0x00002ae4d5a11a58
    2 loadTree() /home/hendrik/workspace/ivb/ivb.c:54 0x0000000000402cbc
    1 main() /home/hendrik/workspace/ivb/ivb.c:23 0x0000000000402a95

    The relevant function is below. (Since the problematic line is in the
    beginning, you might not need all of it but I am really too new in C to
    be able to shorten this. Pointers to a relevant site are welcome.)

    What it does is it takes a file which contains a tree in some funny
    format and constructs the tree in way MONA [1] can handle them.

    Thank you very much.

    [1]: http://www.brics.dk/mona/

    mgTreeNode *loadTree(char *file)
    {
    hcreate(MAX_NODES);

    char symbol[MAX_SYMBOL];
    char name[MAX_NODENAME];
    char left[MAX_NODENAME];
    char right[MAX_NODENAME];
    ENTRY node;
    char* cache[MAX_NODES][3];
    size_t nodeCounter=0;
    size_t i;

    FILE *fp = fopen(file, "r");
    while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol) == 4)
    {
    // construct the node
    mgTreeNode *t = tAlloc();

    // convert the characters to numbers:
    unsigned int len = strlen(symbol);
    t->a = (char*)malloc(len*(sizeof(char))); // freed in freeTree
    for(i = 0; i < len; ++i) {
    t->a = symbol - '0';
    }

    t->left = NULL;
    t->right = NULL;

    // insert it into the hashtable
    node.key = strdup(name); // this memory is freed after hdestroy
    node.data = t;
    hsearch(node, ENTER);

    // insert it into an array
    cache[nodeCounter][0] = node.key;
    cache[nodeCounter][1] = strdup(left); // freed after hdestroy
    cache[nodeCounter][2] = strdup(right); // freed after hdestroy

    ++nodeCounter;
    }
    fclose(fp);

    // loop over the array and fill in the links
    for(i=0; i < nodeCounter; ++i)
    {
    node.key = cache[0];
    ENTRY *n = hsearch(node, FIND);

    node.key = cache[1];
    ENTRY *l = hsearch(node, FIND);

    node.key = cache[2];
    ENTRY *r = hsearch(node, FIND);

    if(l != NULL) ((mgTreeNode *) n->data)->left = (l->data);
    if(r != NULL) ((mgTreeNode *) n->data)->right = (r->data);
    }

    node.key = ROOT_NODE_NAME;
    ENTRY *root = hsearch(node, FIND);

    mgTreeNode *rootNode = NULL;
    if(root != NULL) rootNode = root->data;

    // free the hashtable
    hdestroy();
    // free the keys
    for(i=0; i < nodeCounter; ++i)
    {
    int j;
    for(j=0; j<3; ++j)
    {
    free(cache[j]);
    }
    }
    return rootNode;
    }
    --
    Hendrik Maryns
    http://tcl.sfs.uni-tuebingen.de/~hendrik/
    ==================
    http://aouw.org
    Ask smart questions, get good answers:
    http://www.catb.org/~esr/faqs/smart-questions.html


    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v2.0.4-svn0 (GNU/Linux)
    Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

    iD8DBQFHVZune+7xMGD3itQRAkz/AJ4lhLapj6GsFtG08AkWNjx0ejuU7gCdFz08
    Fbg3Iwrij5ykLmPcwomp24Y=
    =VPbd
    -----END PGP SIGNATURE-----
     
    Hendrik Maryns, Dec 4, 2007
    #1
    1. Advertising

  2. In article <fj4637$ur9$-tuebingen.de>,
    Hendrik Maryns <> wrote:
    >the problem is the following: it causes a segfault. Debugging
    >tells me the following line is the culprit:


    > while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol) ==
    > 4)

    [...]
    > char symbol[MAX_SYMBOL];
    > char name[MAX_NODENAME];
    > char left[MAX_NODENAME];
    > char right[MAX_NODENAME];


    > while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol) == 4)


    That fscanf() can cause an access violation if it encounters
    a line in which the length of any of the strings in the "name" or
    "left" or "right" positions are more than MAX_NODENAME-1 characters long,
    or if the string in the "symbol" position is more than MAX_SYMBOL-1
    characters long. In particular watch out for cases in which the
    string are -exactly- MAX_NODENAME or MAX_SYMBOL long: in that
    case, fscanf() would need to store a trailing '\0' after the MAX_*
    characters had been stored in the variable.
    --
    "Beware of bugs in the above code; I have only proved it correct,
    not tried it." -- Donald Knuth
     
    Walter Roberson, Dec 4, 2007
    #2
    1. Advertising

  3. Hendrik Maryns

    jacob navia Guest

    Hendrik Maryns wrote:
    >
    > [1]: http://www.brics.dk/mona/
    >
    > mgTreeNode *loadTree(char *file)
    > {
    > hcreate(MAX_NODES);
    >
    > char symbol[MAX_SYMBOL];
    > char name[MAX_NODENAME];
    > char left[MAX_NODENAME];
    > char right[MAX_NODENAME];
    > ENTRY node;
    > char* cache[MAX_NODES][3];
    > size_t nodeCounter=0;
    > size_t i;
    >
    > FILE *fp = fopen(file, "r");


    WHAT HAPPENS IF fopen FAILS (the file doesn't exist, or has
    another name, or you have no access to it etc etc???)


    > while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol) == 4)
    > {


    This will trap if fp is NULL because fopen failed.

    --
    jacob navia
    jacob at jacob point remcomp point fr
    logiciels/informatique
    http://www.cs.virginia.edu/~lcc-win32
     
    jacob navia, Dec 4, 2007
    #3
  4. Hendrik Maryns wrote:
    > Hi all,
    >
    > I received a tiny program from a colleague. I want to use it in my Java
    > program. I invoke it with Java’s ProcessBuilder. That not being very
    > relevant, the problem is the following: it causes a segfault. Debugging
    > tells me the following line is the culprit:
    >
    > while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol) == 4)


    It seems that one or more of name, left, right, symbolm is either non
    allocated space of is allocated too little space for the input string or
    fp was not properly assigned.

    [implementation-specific, and non-informative, stack dump removed]

    > mgTreeNode *loadTree(char *file)
    > {
    > hcreate(MAX_NODES);
    >
    > char symbol[MAX_SYMBOL];
    > char name[MAX_NODENAME];
    > char left[MAX_NODENAME];
    > char right[MAX_NODENAME];
    > ENTRY node;
    > char* cache[MAX_NODES][3];
    > size_t nodeCounter=0;
    > size_t i;
    >
    > FILE *fp = fopen(file, "r");


    Did this fopen succeed? If it did not (and you don't check) the
    following fscanf dereferences a null pointer.

    > while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol) == 4)


    If the fopen succeeded, one of MAX_SYMBOL or MAX_NODENAME is too small
    for the input data. The data itself may be munged. This is not a safe
    way to read these strings.
     
    Martin Ambuhl, Dec 4, 2007
    #4
  5. Hendrik Maryns

    John Gordon Guest

    In <fj4637$ur9$-tuebingen.de> Hendrik Maryns <> writes:

    > I received a tiny program from a colleague.


    > What it does is it takes a file which contains a tree in some funny
    > format and constructs the tree in way MONA [1] can handle them.


    > FILE *fp = fopen(file, "r");
    > while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol) == 4)


    My first guess is that it's trying to open the tree file and failing,
    because it can't find it. It then tries to read from the invalid file
    pointer and dumps core.

    Do you have a copy of the tree file? Is is named correctly? Is it located
    in the proper subdirectory? Does it have appropriate permissions for
    reading?

    --
    John Gordon A is for Amy, who fell down the stairs
    B is for Basil, assaulted by bears
    -- Edward Gorey, "The Gashlycrumb Tinies"
     
    John Gordon, Dec 4, 2007
    #5
  6. On Dec 4, 1:25 pm, Hendrik Maryns <> wrote:
    > Hi all,
    >
    > signature.asc
    > 1KDownload
    >
    > I received a tiny program from a colleague. I want to use it in my Java
    > program. I invoke it with Java's ProcessBuilder. That not being very
    > relevant, the problem is the following: it causes a segfault. Debugging
    > tells me the following line is the culprit:
    >
    > while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol) == 4)
    >
    > This is the stack:
    >
    > Thread [0] (Suspended: Signal 'SIGSEGV' received. Description:
    > Segmentation fault.)
    > 4 _IO_vfscanf() 0x00002ae4d5a0565f
    > 3 fscanf() 0x00002ae4d5a11a58
    > 2 loadTree() /home/hendrik/workspace/ivb/ivb.c:54 0x0000000000402cbc
    > 1 main() /home/hendrik/workspace/ivb/ivb.c:23 0x0000000000402a95
    >
    > The relevant function is below. (Since the problematic line is in the
    > beginning, you might not need all of it but I am really too new in C to
    > be able to shorten this. Pointers to a relevant site are welcome.)
    >
    > What it does is it takes a file which contains a tree in some funny
    > format and constructs the tree in way MONA [1] can handle them.
    >
    > Thank you very much.
    >
    > [1]:http://www.brics.dk/mona/
    >
    > mgTreeNode *loadTree(char *file)
    > {
    > hcreate(MAX_NODES);
    >
    > char symbol[MAX_SYMBOL];
    > char name[MAX_NODENAME];
    > char left[MAX_NODENAME];
    > char right[MAX_NODENAME];
    > ENTRY node;
    > char* cache[MAX_NODES][3];
    > size_t nodeCounter=0;
    > size_t i;
    >
    > FILE *fp = fopen(file, "r");
    > while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol) == 4)
    > {
    > // construct the node
    > mgTreeNode *t = tAlloc();
    >
    > // convert the characters to numbers:
    > unsigned int len = strlen(symbol);
    > t->a = (char*)malloc(len*(sizeof(char))); // freed in freeTree
    > for(i = 0; i < len; ++i) {
    > t->a = symbol - '0';
    > }
    >
    > t->left = NULL;
    > t->right = NULL;
    >
    > // insert it into the hashtable
    > node.key = strdup(name); // this memory is freed after hdestroy
    > node.data = t;
    > hsearch(node, ENTER);
    >
    > // insert it into an array
    > cache[nodeCounter][0] = node.key;
    > cache[nodeCounter][1] = strdup(left); // freed after hdestroy
    > cache[nodeCounter][2] = strdup(right); // freed after hdestroy
    >
    > ++nodeCounter;
    > }
    > fclose(fp);
    >
    > // loop over the array and fill in the links
    > for(i=0; i < nodeCounter; ++i)
    > {
    > node.key = cache[0];
    > ENTRY *n = hsearch(node, FIND);
    >
    > node.key = cache[1];
    > ENTRY *l = hsearch(node, FIND);
    >
    > node.key = cache[2];
    > ENTRY *r = hsearch(node, FIND);
    >
    > if(l != NULL) ((mgTreeNode *) n->data)->left = (l->data);
    > if(r != NULL) ((mgTreeNode *) n->data)->right = (r->data);
    > }
    >
    > node.key = ROOT_NODE_NAME;
    > ENTRY *root = hsearch(node, FIND);
    >
    > mgTreeNode *rootNode = NULL;
    > if(root != NULL) rootNode = root->data;
    >
    > // free the hashtable
    > hdestroy();
    > // free the keys
    > for(i=0; i < nodeCounter; ++i)
    > {
    > int j;
    > for(j=0; j<3; ++j)
    > {
    > free(cache[j]);
    > }
    > }
    > return rootNode;}
    >
    > --
    > Hendrik Marynshttp://tcl.sfs.uni-tuebingen.de/~hendrik/
    > ==================http://aouw.org
    > Ask smart questions, get good answers:http://www.catb.org/~esr/faqs/smart-questions.html


    The following links might help in the investigation:

    http://www.eventhelix.com/RealtimeMantra/Basics/debugging_software_crashes.htm

    http://www.eventhelix.com/RealtimeMantra/Basics/debugging_software_crashes_2.htm

    --
    EventStudio - http://www.Eventhelix.com/Eventstudio/
    Sequence diagram based systems engineering tool
     
    EventHelix.com, Dec 5, 2007
    #6
  7. Walter Roberson schreef:
    > In article <fj4637$ur9$-tuebingen.de>,
    > Hendrik Maryns <> wrote:
    >> the problem is the following: it causes a segfault. Debugging
    >> tells me the following line is the culprit:

    >
    >> while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol) ==
    >> 4)

    > [...]
    >> char symbol[MAX_SYMBOL];
    >> char name[MAX_NODENAME];
    >> char left[MAX_NODENAME];
    >> char right[MAX_NODENAME];

    >
    >> while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol) == 4)

    >
    > That fscanf() can cause an access violation if it encounters
    > a line in which the length of any of the strings in the "name" or
    > "left" or "right" positions are more than MAX_NODENAME-1 characters long,
    > or if the string in the "symbol" position is more than MAX_SYMBOL-1
    > characters long. In particular watch out for cases in which the
    > string are -exactly- MAX_NODENAME or MAX_SYMBOL long: in that
    > case, fscanf() would need to store a trailing '\0' after the MAX_*
    > characters had been stored in the variable.


    I suppose that is the reason why one shouldn’t use scanf. However, the
    problem was due to the file not being properly opened.

    Thanks, H.
    --
    Hendrik Maryns
    http://tcl.sfs.uni-tuebingen.de/~hendrik/
    ==================
    http://aouw.org
    Ask smart questions, get good answers:
    http://www.catb.org/~esr/faqs/smart-questions.html


    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v2.0.4-svn0 (GNU/Linux)
    Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

    iD8DBQFHVn71e+7xMGD3itQRAmc1AJ9qefczSKVZXtOT7uT/FrXqBih4igCcCmwP
    uq5bbkM6raKVEXolOPmZRsQ=
    =VXXH
    -----END PGP SIGNATURE-----
     
    Hendrik Maryns, Dec 5, 2007
    #7
  8. Martin Ambuhl schreef:
    > Hendrik Maryns wrote:
    >> Hi all,
    >>
    >> I received a tiny program from a colleague. I want to use it in my Java
    >> program. I invoke it with Java’s ProcessBuilder. That not being very
    >> relevant, the problem is the following: it causes a segfault. Debugging
    >> tells me the following line is the culprit:
    >>
    >> while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol)
    >> == 4)

    >
    > It seems that one or more of name, left, right, symbolm is either non
    > allocated space of is allocated too little space for the input string or
    > fp was not properly assigned.
    >
    > [implementation-specific, and non-informative, stack dump removed]


    Please have some patience here. I am a regular poster to c.l.java.*,
    and now how annoying it can be to answer newbie questions. That’s why I
    stressed that twice.

    >> mgTreeNode *loadTree(char *file)
    >> {
    >> hcreate(MAX_NODES);
    >>
    >> char symbol[MAX_SYMBOL];
    >> char name[MAX_NODENAME];
    >> char left[MAX_NODENAME];
    >> char right[MAX_NODENAME];
    >> ENTRY node;
    >> char* cache[MAX_NODES][3];
    >> size_t nodeCounter=0;
    >> size_t i;
    >>
    >> FILE *fp = fopen(file, "r");

    >
    > Did this fopen succeed? If it did not (and you don't check) the
    > following fscanf dereferences a null pointer.


    This indeed was the problem: I gave the wrong arguments to the debug
    program.

    How do I properly check whether the open succeeded? Is
    if (fp == 0) exit(-2);
    enough?

    >> while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol)
    >> == 4)

    >
    > If the fopen succeeded, one of MAX_SYMBOL or MAX_NODENAME is too small
    > for the input data. The data itself may be munged. This is not a safe
    > way to read these strings.


    I suppose. Once again: I didn’t write this code, I only want to use it,
    for now; until I get time to write it properly.

    Thanks, H.
    --
    Hendrik Maryns
    http://tcl.sfs.uni-tuebingen.de/~hendrik/
    ==================
    http://aouw.org
    Ask smart questions, get good answers:
    http://www.catb.org/~esr/faqs/smart-questions.html


    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v2.0.4-svn0 (GNU/Linux)
    Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

    iD8DBQFHVn/Ee+7xMGD3itQRAgCiAJ9smijHUAnNPPXY7nxjYr7hihHvswCdHk1z
    EVXol1EhCkP1x+gSEQybmG8=
    =81gK
    -----END PGP SIGNATURE-----
     
    Hendrik Maryns, Dec 5, 2007
    #8
  9. Hendrik Maryns

    santosh Guest

    Hendrik Maryns wrote:

    > Martin Ambuhl schreef:


    <snip>

    >>> FILE *fp = fopen(file, "r");

    >>
    >> Did this fopen succeed? If it did not (and you don't check) the
    >> following fscanf dereferences a null pointer.

    >
    > This indeed was the problem: I gave the wrong arguments to the debug
    > program.
    >
    > How do I properly check whether the open succeeded? Is
    > if (fp == 0) exit(-2);
    > enough?


    fopen() returns a NULL pointer if it fails. So you check like:

    FILE *f = fopen("file.txt" "r");
    if (f == NULL) { puts("fopen() failed."); /* whatever */ }
    else { /* proceed */ }

    <snip>
     
    santosh, Dec 5, 2007
    #9
  10. santosh schreef:
    > Hendrik Maryns wrote:
    >
    >> Martin Ambuhl schreef:

    >
    > <snip>
    >
    >>>> FILE *fp = fopen(file, "r");
    >>> Did this fopen succeed? If it did not (and you don't check) the
    >>> following fscanf dereferences a null pointer.

    >> This indeed was the problem: I gave the wrong arguments to the debug
    >> program.
    >>
    >> How do I properly check whether the open succeeded? Is
    >> if (fp == 0) exit(-2);
    >> enough?

    >
    > fopen() returns a NULL pointer if it fails. So you check like:
    >
    > FILE *f = fopen("file.txt" "r");
    > if (f == NULL) { puts("fopen() failed."); /* whatever */ }
    > else { /* proceed */ }


    Thanks. I’m getting the hang of this, I think. I prefer to write such
    stuff to stderr though, so I did the following:

    FILE *fp = fopen(file, "r");
    if (fp == NULL) {
    fputs("Opening of the tree file failed. Did you enter it correctly
    and do you have read access?\n", stderr);
    exit(-2);
    }

    Cheers, H.
    --
    Hendrik Maryns
    http://tcl.sfs.uni-tuebingen.de/~hendrik/
    ==================
    http://aouw.org
    Ask smart questions, get good answers:
    http://www.catb.org/~esr/faqs/smart-questions.html


    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v2.0.4-svn0 (GNU/Linux)
    Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

    iD8DBQFHVppVe+7xMGD3itQRAm2nAJ41SxVrkMoW3iuNJpO9ScPehSALmQCZAW/+
    yagQaTQKHXM+HSzF5Dve1gY=
    =n2n5
    -----END PGP SIGNATURE-----
     
    Hendrik Maryns, Dec 5, 2007
    #10
  11. Hendrik Maryns

    santosh Guest

    Hendrik Maryns wrote:

    > santosh schreef:
    >> Hendrik Maryns wrote:
    >>
    >>> Martin Ambuhl schreef:

    >>
    >> <snip>
    >>
    >>>>> FILE *fp = fopen(file, "r");
    >>>> Did this fopen succeed? If it did not (and you don't check) the
    >>>> following fscanf dereferences a null pointer.
    >>> This indeed was the problem: I gave the wrong arguments to the debug
    >>> program.
    >>>
    >>> How do I properly check whether the open succeeded? Is
    >>> if (fp == 0) exit(-2);
    >>> enough?

    >>
    >> fopen() returns a NULL pointer if it fails. So you check like:
    >>
    >> FILE *f = fopen("file.txt" "r");
    >> if (f == NULL) { puts("fopen() failed."); /* whatever */ }
    >> else { /* proceed */ }

    >
    > Thanks. I?m getting the hang of this, I think. I prefer to write
    > such stuff to stderr though, so I did the following:
    >
    > FILE *fp = fopen(file, "r");
    > if (fp == NULL) {
    > fputs("Opening of the tree file failed. Did you enter it
    > correctly
    > and do you have read access?\n", stderr);
    > exit(-2);
    > }


    One nit. The only portable exit status values that the C Standard
    describes are 0, EXIT_SUCCESS and EXIT_FAILURE. The latter two are
    defined in stdlib.h header. The value -2 may have meaning on some
    systems, but may cause undefined behaviour on others.
     
    santosh, Dec 5, 2007
    #11
  12. Hendrik Maryns

    James Kuyper Guest

    Hendrik Maryns wrote:
    > Martin Ambuhl schreef:
    >> Hendrik Maryns wrote:

    ....
    >>> mgTreeNode *loadTree(char *file)
    >>> {
    >>> hcreate(MAX_NODES);
    >>>
    >>> char symbol[MAX_SYMBOL];
    >>> char name[MAX_NODENAME];
    >>> char left[MAX_NODENAME];
    >>> char right[MAX_NODENAME];
    >>> ENTRY node;
    >>> char* cache[MAX_NODES][3];
    >>> size_t nodeCounter=0;
    >>> size_t i;
    >>>
    >>> FILE *fp = fopen(file, "r");

    >> Did this fopen succeed? If it did not (and you don't check) the
    >> following fscanf dereferences a null pointer.

    >
    > This indeed was the problem: I gave the wrong arguments to the debug
    > program.
    >
    > How do I properly check whether the open succeeded? Is
    > if (fp == 0) exit(-2);
    > enough?


    There are only three things you can portably pass to exit():
    EXIT_SUCCESS, EXIT_FAILURE (both #defined in <stdlib.h> or 0. Note: not
    all programs need to be portable, but you should write non-portable code
    only by reason of a deliberate decision, not as a matter of habit.

    There's no guarantee that EXIT_SUCCESS is 0, nor is there any guarantee
    that EXIT_SUCCESS is not 0. It can be arguably be inferred that
    EXIT_FAILURE must be different from both 0 and EXIT_SUCCESS.

    I normally would add a perror(file) call before the exit() call, for
    diagnostic purposes, but only if it's acceptable for your program to
    send something to stderr.

    I would also recommend avoiding direct calls to exit(). In most cases a
    subroutine should report a problem to it's caller, allowing the caller
    to perform any cleanup that may be needed.
     
    James Kuyper, Dec 5, 2007
    #12
  13. On 5 Dec, 12:32, Hendrik Maryns <> wrote:
    > santosh schreef:
    > > Hendrik Maryns wrote:


    > >> How do I properly check whether the open succeeded? Is
    > >> if (fp == 0) exit(-2);
    > >> enough?

    >
    > > fopen() returns a NULL pointer if it fails. So you check like:

    >
    > > FILE *f = fopen("file.txt" "r");
    > > if (f == NULL) { puts("fopen() failed."); /* whatever */ }
    > > else { /* proceed */ }

    >
    > Thanks. I'm getting the hang of this, I think. I prefer to write such
    > stuff to stderr though, so I did the following:
    >
    > FILE *fp = fopen(file, "r");
    > if (fp == NULL) {
    > fputs("Opening of the tree file failed. Did you enter it correctly
    > and do you have read access?\n", stderr);
    > exit(-2);
    >
    > }


    Much better is:

    if( fp == NULL ) {
    perror( file );
    exit( EXIT_FAILURE );
    }

    Note that not all implementations will properly set errno
    when fopen fails, so you might prefer:

    if( fp == NULL ) {
    fprintf( stderr, "Error opening %s: %s\n", file,
    strerror( errno ));
    exit( EXIT_FAILURE );
    }

    This will give you nicer messages explaining the reason. Eg:

    $ ./a.out bar
    bar: No such file or directory
    $ ./a.out foo
    foo: Permission denied

    It's always a good idea to use the error message that have
    been kindly provided by the implementation rather than
    trying to role your own.
     
    William Pursell, Dec 5, 2007
    #13
  14. jacob navia <> writes:
    > Hendrik Maryns wrote:

    [...]
    >> mgTreeNode *loadTree(char *file)
    >> {

    [...]
    >> FILE *fp = fopen(file, "r");

    >
    > WHAT HAPPENS IF fopen FAILS (the file doesn't exist, or has
    > another name, or you have no access to it etc etc???)
    >
    >
    >> while(fscanf(fp, "%s -> %s -> %s -> %s", name, left, right, symbol) == 4)
    >> {

    >
    > This will trap if fp is NULL because fopen failed.


    It *might* trap. It *will* invoke undefined behavior.

    (It may well be the case that it will "trap", whatever that happens to
    mean, on most or all existing implementations, but that's not
    guaranteed.)

    --
    Keith Thompson (The_Other_Keith) <>
    Looking for software development work in the San Diego area.
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Dec 5, 2007
    #14
  15. William Pursell schreef:
    > On 5 Dec, 12:32, Hendrik Maryns <> wrote:
    >> santosh schreef:
    >>> Hendrik Maryns wrote:

    >
    >>>> How do I properly check whether the open succeeded? Is
    >>>> if (fp == 0) exit(-2);
    >>>> enough?
    >>> fopen() returns a NULL pointer if it fails. So you check like:
    >>> FILE *f = fopen("file.txt" "r");
    >>> if (f == NULL) { puts("fopen() failed."); /* whatever */ }
    >>> else { /* proceed */ }

    >> Thanks. I'm getting the hang of this, I think. I prefer to write such
    >> stuff to stderr though, so I did the following:
    >>
    >> FILE *fp = fopen(file, "r");
    >> if (fp == NULL) {
    >> fputs("Opening of the tree file failed. Did you enter it correctly
    >> and do you have read access?\n", stderr);
    >> exit(-2);
    >>
    >> }

    >
    > Much better is:
    >
    > if( fp == NULL ) {
    > perror( file );
    > exit( EXIT_FAILURE );
    > }


    What is ‘file’ here? Should I replace it by stderr? (I am doing no
    logging to a file, it is really a tiny app.)

    > Note that not all implementations will properly set errno
    > when fopen fails, so you might prefer:
    >
    > if( fp == NULL ) {
    > fprintf( stderr, "Error opening %s: %s\n", file,
    > strerror( errno ));
    > exit( EXIT_FAILURE );
    > }


    I get the following compiler error here:

    error: ‘errno’ undeclared (first use in this function)

    It is a bit unclear which part of the code you provide can be copied
    over, and which parts I have to replace by meaningful variables. Or do
    I simply have to declare errno somewhere?

    > This will give you nicer messages explaining the reason. Eg:
    >
    > $ ./a.out bar
    > bar: No such file or directory
    > $ ./a.out foo
    > foo: Permission denied
    >
    > It's always a good idea to use the error message that have
    > been kindly provided by the implementation rather than
    > trying to role your own.


    Of course. Only that I didn’t know about them. I think I really have
    to go and read some tutorials now, before trying to enhance other
    people’s code in a language I am not familiar with.

    Thank you all for your kind help.

    H.
    --
    Hendrik Maryns
    http://tcl.sfs.uni-tuebingen.de/~hendrik/
    ==================
    http://aouw.org
    Ask smart questions, get good answers:
    http://www.catb.org/~esr/faqs/smart-questions.html


    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v2.0.4-svn0 (GNU/Linux)
    Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

    iD8DBQFHV8p3e+7xMGD3itQRAgs7AJ93UE6BSP3TzQZpTUKlBp4OWwhVywCcDW70
    hmUUfB3e+8eoZJMoVukkPIA=
    =6cyT
    -----END PGP SIGNATURE-----
     
    Hendrik Maryns, Dec 6, 2007
    #15
  16. James Kuyper schreef:
    > Hendrik Maryns wrote:
    >> Martin Ambuhl schreef:
    >>> Hendrik Maryns wrote:

    > ...
    >>>> mgTreeNode *loadTree(char *file)
    >>>> {
    >>>> hcreate(MAX_NODES);
    >>>>
    >>>> char symbol[MAX_SYMBOL];
    >>>> char name[MAX_NODENAME];
    >>>> char left[MAX_NODENAME];
    >>>> char right[MAX_NODENAME];
    >>>> ENTRY node;
    >>>> char* cache[MAX_NODES][3];
    >>>> size_t nodeCounter=0;
    >>>> size_t i;
    >>>>
    >>>> FILE *fp = fopen(file, "r");
    >>> Did this fopen succeed? If it did not (and you don't check) the
    >>> following fscanf dereferences a null pointer.

    >>
    >> This indeed was the problem: I gave the wrong arguments to the debug
    >> program.
    >>
    >> How do I properly check whether the open succeeded? Is
    >> if (fp == 0) exit(-2);
    >> enough?

    >
    > There are only three things you can portably pass to exit():
    > EXIT_SUCCESS, EXIT_FAILURE (both #defined in <stdlib.h> or 0. Note: not
    > all programs need to be portable, but you should write non-portable code
    > only by reason of a deliberate decision, not as a matter of habit.


    I totally agree. However, I use the exit value in the calling program
    (in Java). And there, 1 has the special meaning: ACCEPT. So I would
    not want to return 1, to which EXIT_FAILURE is defined. I guess I will
    make it a little bit more non-portable for now, until I read up on JNI
    and can avoid using the exit value by calling the relevant functions
    directly from my Java program. But I’ll definitely remember this.

    > There's no guarantee that EXIT_SUCCESS is 0, nor is there any guarantee
    > that EXIT_SUCCESS is not 0. It can be arguably be inferred that
    > EXIT_FAILURE must be different from both 0 and EXIT_SUCCESS.
    >
    > I normally would add a perror(file) call before the exit() call, for
    > diagnostic purposes, but only if it's acceptable for your program to
    > send something to stderr.
    >
    > I would also recommend avoiding direct calls to exit(). In most cases a
    > subroutine should report a problem to it's caller, allowing the caller
    > to perform any cleanup that may be needed.


    Good point. I’m so used to Java’s automatic garbage collection that I
    forgot about this. But even there it is better to throw an exception or
    something.
    However, if the process is totally dead, won’t the OS reclaim the memory?

    Thanks, H.
    --
    Hendrik Maryns
    http://tcl.sfs.uni-tuebingen.de/~hendrik/
    ==================
    http://aouw.org
    Ask smart questions, get good answers:
    http://www.catb.org/~esr/faqs/smart-questions.html


    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v2.0.4-svn0 (GNU/Linux)
    Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

    iD8DBQFHV830e+7xMGD3itQRAtgTAJ42Pz90+7Dk/tIQzI52v2lTKSWBAwCffxt4
    Y9nZmTQh7xEEzOoh+CA8PFY=
    =KvVZ
    -----END PGP SIGNATURE-----
     
    Hendrik Maryns, Dec 6, 2007
    #16
  17. Hendrik Maryns

    James Kuyper Guest

    Hendrik Maryns wrote:
    > William Pursell schreef:
    >> On 5 Dec, 12:32, Hendrik Maryns <> wrote:

    ....
    >>> FILE *fp = fopen(file, "r");
    >>> if (fp == NULL) {
    >>> fputs("Opening of the tree file failed. Did you enter it correctly
    >>> and do you have read access?\n", stderr);
    >>> exit(-2);
    >>>
    >>> }

    >> Much better is:
    >>
    >> if( fp == NULL ) {
    >> perror( file );
    >> exit( EXIT_FAILURE );
    >> }

    >
    > What is ‘file’ here? Should I replace it by stderr? (I am doing no
    > logging to a file, it is really a tiny app.)


    The variable named 'file' is the same one here that it was in the call
    to fopen().

    >> Note that not all implementations will properly set errno
    >> when fopen fails, so you might prefer:
    >>
    >> if( fp == NULL ) {
    >> fprintf( stderr, "Error opening %s: %s\n", file,
    >> strerror( errno ));
    >> exit( EXIT_FAILURE );
    >> }

    >
    > I get the following compiler error here:
    >
    > error: ‘errno’ undeclared (first use in this function)


    You need to add

    #include <errno.h>

    before using either perror() or errno.
     
    James Kuyper, Dec 6, 2007
    #17
  18. Hendrik Maryns

    James Kuyper Guest

    Hendrik Maryns wrote:
    > James Kuyper schreef:

    ....
    >> I would also recommend avoiding direct calls to exit(). In most cases a
    >> subroutine should report a problem to it's caller, allowing the caller
    >> to perform any cleanup that may be needed.

    >
    > Good point. I’m so used to Java’s automatic garbage collection that I
    > forgot about this. But even there it is better to throw an exception or
    > something.
    > However, if the process is totally dead, won’t the OS reclaim the memory?


    Yes, but relying upon that fact is a sign of sloppy programming. There's
    often something that can be done about a failure condition that is more
    useful than simply calling exit(). Avoiding the use of exit() helps make
    it easier for you to identify those situations.
     
    James Kuyper, Dec 6, 2007
    #18
  19. Hendrik Maryns

    CBFalconer Guest

    Hendrik Maryns wrote:
    >

    .... snip ...
    >
    > I get the following compiler error here:
    >
    > error: ‘errno’ undeclared (first use in this function)


    Just #include <errno.h>. All perfectly standard.

    --
    Chuck F (cbfalconer at maineline dot net)
    <http://cbfalconer.home.att.net>
    Try the download section.



    --
    Posted via a free Usenet account from http://www.teranews.com
     
    CBFalconer, Dec 6, 2007
    #19
  20. Hendrik Maryns

    CBFalconer Guest

    Hendrik Maryns wrote:
    > James Kuyper schreef:
    >

    .... snip ...
    >
    >> There are only three things you can portably pass to exit():
    >> EXIT_SUCCESS, EXIT_FAILURE (both #defined in <stdlib.h> or 0.
    >> Note: not all programs need to be portable, but you should write
    >> non-portable code only by reason of a deliberate decision, not as
    >> a matter of habit.

    >
    > I totally agree. However, I use the exit value in the calling
    > program (in Java). And there, 1 has the special meaning: ACCEPT.
    > So I would not want to return 1, to which EXIT_FAILURE is defined.
    > I guess I will make it a little bit more non-portable for now,
    > until I read up on JNI and can avoid using the exit value by
    > calling the relevant functions directly from my Java program.
    > But I’ll definitely remember this.


    Sounds like IBM hardware. In general the exit status value
    returned (in C) is transformed by the run-time into something
    suitable for the actual system. Stick with the C standards.

    --
    Chuck F (cbfalconer at maineline dot net)
    <http://cbfalconer.home.att.net>
    Try the download section.



    --
    Posted via a free Usenet account from http://www.teranews.com
     
    CBFalconer, Dec 6, 2007
    #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. Joel
    Replies:
    4
    Views:
    9,316
    John Harrison
    Oct 11, 2004
  2. rh0dium
    Replies:
    2
    Views:
    476
    Adriano Ferreira
    May 20, 2005
  3. =?Utf-8?B?R3V5IFRob3JudG9u?=

    Needing some help with Business Objects in ASP.Net PLEASE

    =?Utf-8?B?R3V5IFRob3JudG9u?=, Nov 9, 2006, in forum: ASP .Net
    Replies:
    6
    Views:
    340
    =?Utf-8?B?R3V5IFRob3JudG9u?=
    Nov 9, 2006
  4. Elinore

    newbie Q, segmentation fault

    Elinore, Mar 3, 2005, in forum: C Programming
    Replies:
    4
    Views:
    376
    CBFalconer
    Mar 4, 2005
  5. Zheng Da
    Replies:
    3
    Views:
    11,629
Loading...

Share This Page