Program and Dr. Watson crashes for unknown reasons...

Discussion in 'C Programming' started by Steven, Feb 24, 2005.

  1. Steven

    Steven Guest

    The following program runs alright until the return statement, where it
    crashes, and consequently, Dr. Watson (drwtsn32.exe) also crashes.
    Deleting the call to fgets() and everything runs fine. I *think* the
    stack or something got mess up during the fgets(). Could anyone help?

    Thanks.

    #include <stdio.h>
    #include <stdlib.h>

    int main(int argc, char *argv[]){
    char **fbuffer=(char **)malloc(8192*sizeof(char));
    FILE *fp=fopen("data.dat","r");
    fgets(fbuffer[count],64,fp); //Trouble here.
    printf("%s",fbuffer[count]);
    system("pause");
    return 0;
    }
    //(data.dat contains 2 short lines of text)
     
    Steven, Feb 24, 2005
    #1
    1. Advertisements

  2. Steven

    Artie Gold Guest

    Question 1: What is `count'?
    Question 2: Assuming `count' is defined somewhere, what does
    `fbuffer[count]' point to? [Hint, no memory has been allocated to it.]
    HTH,
    --ag
     
    Artie Gold, Feb 24, 2005
    #2
    1. Advertisements

  3. Steven

    Steven Guest

    Question 1: What is `count'?
    it.]

    Sorry, the original count was used in a loop, and when I posted this
    message I deleted the loop. So just assume count is 0. (In the
    original source count was used in a loop and took on the value from
    0-1)

    fbuffer[count] *should* be initialized by the fgets() command, since it
    stores the pointer to the string to the pointer specified in the first
    argument.

    Like I said, the program runs fine. It displays the right string.
    Just not returning correctly.

    Also, the first line of the code after int main() allocates fbuffer.
     
    Steven, Feb 24, 2005
    #3
  4. Steven

    Jarmo Guest

    It doesn't run fine, you just think it does. You've corrupted the heap and,
    in this specific case but not generally, you don't find out about it until
    you terminate.

    I'm guessing that you're trying to allocate enough space to hold 128 lines
    of 64 character each, right? Why not just allocate for one line and then
    re-use that as you go along?
     
    Jarmo, Feb 24, 2005
    #4
  5. Steven

    Artie Gold Guest

    It allocates memory to fbuffer. It does *not* allocate memory to the
    pointers *in* fbuffer (which is a pointer-to-pointer-of-char). You need
    to allocate memory for each line.

    HTH,
    --ag
     
    Artie Gold, Feb 24, 2005
    #5
  6. You've allocated memory for 8192 pointers-to-char. Or you have
    attempted to; you haven't checked that the return from malloc()
    was not NULL. That could be a problem in production code, but
    it's not the real issue here. Likewise with the fact that you've
    added a redundant cast "(char **)" to the return from malloc,
    which, as a void *pointer is already ready for action in any
    capacity.

    The most important problems with the code are two-fold.

    1) Those little double asterisks ("**") mean that you're
    dealing with a pointer-to-pointer-to-char. I presume you
    intend to allocate 8192 pointers-to-char, but in that case
    the size you have given to malloc is wrong. In the C language
    sizeof(char) is, by definition, 1. But the size of a pointer
    to char is not 1 by definition, and in practical contexts is
    almost sure to be greater than 1.

    You should say:

    char **fbuffer = malloc(8192 * sizeof *fbuffer);

    (or, "give me 8192 of whatever fbuffer points to, please").

    2) Even supposing you'd succeeded in allocating 8192 pointers
    to char, each and every one of them is pointing nowhere, and is
    not ready to receive any char data. If you want to use the
    fgets() construction you have shown, then you have to first
    point each of your pointers-to-char to a 64-byte arena, with
    something like

    int i;

    for (i=0; i<8192; i++) {
    fbuffer = malloc(64);
    }

    3) There's also the small matter of count's being undeclared.
    I presume it is an int, initialized to 0 and incremented
    after each call to fgets().

    Or just maybe you don't really want 8192 pointers-to-char after
    all. In that case, lose the "**".

    FILE *fp;
    char *buf;

    fp = fopen("data.dat","r"); /* check return */
    buf = malloc(64); /* check return */

    fgets(buf, 64, fp);
    printf("%s", buf);

    Allin Cottrell
     
    Allin Cottrell, Feb 24, 2005
    #6
  7. Please preserve the context (your program) when replying. Usenet
    is quirky and not everyone will receive your original post.
    The fgets() function won't initialise the pointer. It can't even
    if it wanted to since the pointer is passed by value, not by
    reference.

    Having allocated an array of char pointers, you need to initialise
    those pointers yourself.
    What you're seeing is a classic example of 'undefined behaviour'.
    Once your program invokes undefined behaviour, then it can do
    anything, including giving the illusion that everything is just
    fine until main returns.
    It allocates the array of pointers only.
     
    Peter Nilsson, Feb 24, 2005
    #7
  8. Steven

    Steven Guest

    Thanks to everybody who replied to the topic. I totally forgot to
    allocate memory to each *fbuffer elements.

    Just added
    for(count=64;count!=0;count--)
    fbuffer[count]=(char *)malloc(64*sizeof(char));
    and changed
    char **fbuffer=(char **)malloc(8192*sizeof(char));
    to
    char **fbuffer=(char **)malloc(128*sizeof(*fbuffer));

    It works fine now.
    I omitted the error catching code to shorten the post.
     
    Steven, Feb 24, 2005
    #8
  9. Steven

    CBFalconer Guest

    And you should omit the useless error message suppressing casts.
    Also sizeof(char) is 1 by definition, and only serves to confuse.
    The use of magic numbers 64 and 128 is suspicious.
     
    CBFalconer, Feb 24, 2005
    #9
  10. Steven

    Steven Guest

    It is a habit, makes code clearer.
    64 and 128 are the closest powers of two to 50 and 100. I don't know
    why it is suspicious...
     
    Steven, Feb 27, 2005
    #10
  11. Not in something that claims to be a C program it doesn't.
    The use of any number other than 0 or 1 is suspicious, and I'm not so
    sure about 1. In a non-trivial program, how do you know that this
    '64' represents something different to (or the same as) that other
    '64'? You don't.

    mlp
     
    Mark L Pappin, Feb 27, 2005
    #11
    1. Advertisements

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.