Thread: This runs..but why not this. Help me to understand, please

  1. #1
    Registered User
    Join Date
    Jul 2008
    Posts
    8

    Question This runs..but why not this. Help me to understand, please

    Hi, here is my problem...I created a program that finds the average of a user defined amount of numbers. It is a homework exercise to learn about dynamic memory (aka The heap), and the functions malloc() and free() .

    This is what it does:

    1)Asks user "How many numbers do you want to average ?".

    2)Allocates just enough memory using malloc() to store all numbers + the average in an
    int array.

    3)Asks user for each number to be averaged, and stores them on the heap, also their total sum is stored in the last array block on the heap

    4)The total sum is divided by the amount of numbers entered to get the average.Which is store where the total sum was stored(as the last array on the heap). total sum /= total amount of numbers.


    5)print all numbers entered and their average.

    6)wait for a character return, and then end the program


    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    
    
    main()
    {
       int   *integers;
       int   num1;
       int   i;
       
       
       printf("how many numbers do you want to average: ");
       scanf("%d", &num1);
       
       integers = malloc(num1 * sizeof(int));
       if(integers == NULL)
       {
          printf("Malloc failed\n");
          exit(1);
       }
       
       integers[num1] = 0;
       
       for(i = 0; i < num1; i++)
       {
          printf("#%d Please enter a number: ", i + 1);
          scanf("%d", &integers[i]);
          integers[num1] += integers[i];
       }
       
       integers[num1] = integers[num1] / num1;
       
       printf("The average of ");
       for(i = 0; i < num1; i++)
          printf("%d ", integers[i]);
          
       printf("is: %d\n", integers[num1]);
       
       free(integers);
    
       fflush(stdin);
       getchar();
    }

    It works great, homework exercise #2 officially : complete.
    Until that is, i get it in to my head that, as a trivial (self imposed) academic exercise, i should break it up in to functions.Here it is:

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    
    int allocate_memory(int *integers, int *num1);
    int average_of(int *integers, int *num1);
    int print_average(int *integers, int *num1);
    
    
    allocate_memory(int *integers, int *num1)
    {
        printf("How many numbers do you want to average: ");
        scanf("%d", num1);
        
        integers = malloc(*num1 * sizeof(int));
        
        if(integers == NULL)
          {
             printf("malloc has failed\n");
             exit(1);
          }
    }
    
    average_of(int *integers, int *num1)
    {
       int    i;
    
       integers[*num1] = 0;
    
       for(i = 0; i < *num1; i++)
          {
             printf("#%d Please enter a number: ", i + 1);
             scanf("%d", &integers[i]);
             integers[*num1] += integers[i];
          }
       integers[*num1] /= *num1;
    }
    
    print_average(int *integers, int *num1)
    {
       int    i;
       
       printf("The average of ");
       
       for(i = 0; i < *num1; i++)
          printf("%d ", integers[i]);
          
       printf("is : %d\n", integers[*num1]);
    }
    
    main()
    {
       int   *integers;
       int   num1;
       
      
       allocate_memory(integers, &num1);
       average_of(integers, &num1);
       print_average(integers, &num1);
       
       free(integers);
       
       fflush(stdin);
       getchar();
    }

    This compiles!!!
    But when i run it, it opens a console window, and prints "How many numbers do you want to average: ". Then, after you enter a number, it crashes. I can't figure it out... all day i've tried. I don't understand...am i not using pointers correctly? Why does the first program work, but not the second. Please, any help is appreciated.
    I'm still learning C so, i apologize in advance if my question seems stupid to anyone.


    If you can't explain it simply, you don't understand it well enough.
    Albert Einstein

  2. #2
    Deathray Engineer MacGyver's Avatar
    Join Date
    Mar 2007
    Posts
    3,210
    Few things to note:

    In the first program:

    Code:
    integers[num1] = 0;
    This is accessing memory you don't own from malloc(). It might have worked because malloc() ended up allocating more than enough memory, but such code can result in a crash.

    Also, main() should explicitly return an int and be defined as such for the latest C standard.

    fflush(stdin) should be avoided.

    Now as far as your new program goes, read this topic: http://cboard.cprogramming.com/showthread.php?t=105380

    I just explained the same concept today about why memory allocation in this manner fails.

  3. #3
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Functions are like Vegas: what happens in a function, stays in the function. Just because you change the value of integers inside the function allocate_memory, doesn't mean that you can see that change elsewhere in the program.

    Your allocate_memory doesn't return anything at the moment; maybe you could make it return the pointer that was malloc'ed, so that it would get called as integers = allocate_memory(7) or whatever.

  4. #4
    Registered User
    Join Date
    Dec 2007
    Posts
    2,675
    Off-topic: tabstop, I had to steal that for my sig. With proper attribution, of course

  5. #5
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Chappell Hill, Texas
    Posts
    2,332
    Quote Originally Posted by rags_to_riches View Post
    Off-topic: tabstop, I had to steal that for my sig. With proper attribution, of course
    My first thought went back to my first car - a '73 Chevy Vega. And yes, what happened in that Vega, stayed in that Vega.
    Mainframe assembler programmer by trade. C coder when I can.

  6. #6
    Registered User
    Join Date
    Jul 2008
    Posts
    8
    Functions are like Vegas: what happens in a function, stays in the function.
    I know this.
    However, recently i learned that pointers can be past in to functions via parameters, and whatever happens to/in that address in the function is retained after the function ends.


    Tapstop your suggestion fixes everything... for some reason that i don't fully understand, it fixes everything. In allocate_memory() instead of just passing my malloc() to integers, i do an explicit return integers;, and i changed my function to return a pointer. In main() I also entered: integers = allocate_memory(integers, &num1);
    check it out:

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    
    int *allocate_memory(int *integers, int *num1);
    int average_of(int *integers, int *num1);
    int print_average(int *integers, int *num1);
    
    
    *allocate_memory(int *integers, int *num1)
    {
        printf("How many numbers do you want to average: ");
        scanf("%d", num1);
        
        integers = malloc(*num1 * sizeof(int));
        
        if(integers == NULL)
          {
             printf("malloc has failed\n");
             exit(1);
          }
        
        return integers;
    }
    
    
    average_of(int *integers, int *num1)
    {
       int    i;
    
       integers[*num1] = 0;
    
       for(i = 0; i < *num1; i++)
          {
             printf("#%d Please enter a number: ", i + 1);
             scanf("%d", &integers[i]);
             integers[*num1] += integers[i];
          }
       integers[*num1] /= *num1;
    }
    
    print_average(int *integers, int *num1)
    {
       int    i;
       
       printf("The average of ");
       
       for(i = 0; i < *num1; i++)
          printf("%d ", integers[i]);
          
       printf("is : %d\n", integers[*num1]);
    }
    
    main()
    {
       int   *integers;
       int   num1;
       
      
       integers = allocate_memory(integers, &num1);
       average_of(integers, &num1);
       print_average(integers, &num1);
       
       free(integers);
       
       fflush(stdin);
       getchar();
    }


    This runs perfectly. I don't get it though, why didn't the original work. Why did i have to explictly return the address of integers if thats what i already set malloc() to?

    macgyver: i don't understand what your other post means...i'm just a beginner. http://cboard.cprogramming.com/showthread.php?t=105380

  7. #7
    Deathray Engineer MacGyver's Avatar
    Join Date
    Mar 2007
    Posts
    3,210
    It means you can't change a pointer's value in a function, as tabstop was also saying. To change it, you must pass the address of the pointer (ie. a pointer to a pointer), or use his approach of returning a pointer.

  8. #8
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    You can change what is pointed to by a function parameter, but you can never change the thing itself. So in your example, integers was a pointer to some integers -- those integers could be changed, because they were pointed to, but the value of integers itself could not be changed -- integers could not be set to point to a new bunch of integers.

    Edit: That's a really confusing name, so maybe the typeface will help.
    Last edited by tabstop; 07-20-2008 at 09:52 PM.

  9. #9
    Registered User
    Join Date
    Jul 2008
    Posts
    8
    MacGyver: Is this what you meant by
    you must pass the address of the pointer (ie. a pointer to a pointer)


    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    
    int allocate_memory(int *integers, int *num1);
    int average_of(int *integers, int *num1);
    int print_average(int *integers, int *num1);
    
    
    allocate_memory(int *integers, int *num1)
    {
    
                
        printf("How many numbers do you want to average: ");
        scanf("%d", num1);
        
        integers = malloc(*num1 * sizeof(int));
        
        if(integers == NULL)
          {
             printf("malloc has failed\n");
             exit(1);
          }
        
        
    }
    
    
    average_of(int *integers, int *num1)
    {
       int    i;
    
       integers[*num1] = 0;
    
       for(i = 0; i < *num1; i++)
          {
             printf("#%d Please enter a number: ", i + 1);
             scanf("%d", &integers[i]);
             integers[*num1] += integers[i];
          }
       integers[*num1] /= *num1;
    }
    
    print_average(int *integers, int *num1)
    {
       int    i;
       
       printf("The average of ");
       
       for(i = 0; i < *num1; i++)
          printf("%d ", integers[i]);
          
       printf("is : %d\n", integers[*num1]);
    }
    
    main()
    {
       int   *integers;
       int   num1;
       int *ptr; 
       integers = ptr;
      
       allocate_memory(integers, &num1);
       average_of(integers, &num1);
       print_average(integers, &num1);
       
       free(integers);
       
       fflush(stdin);
       getchar();
    }
    This code runs flawlessly...all i did was make integers a pointer to a new pointer named ptr.
    int *ptr;
    integers = ptr;
    i just can't seem to picture how this line is allowing the pointer integers to escape from the allocate_memory() function. Now integers is initialized to a pointer (ptr) that points to a random address and is never used...except maybe threw integers???? sorry about my lame variable names

  10. #10
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by polymatrix View Post

    This code runs flawlessly...
    Code:
    How many numbers do you want to average: 7
    #1 Please enter a number: 3
    
    Process returned -1073741819
    Definitely "flawless"?

  11. #11
    Registered User
    Join Date
    Jul 2008
    Posts
    8
    I compiled it on Dev-C++ 4.9.9.2
    It runs flawlessly
    How many numbers do you want to average: 3
    #1 Please enter a number: 78
    #2 Please enter a number: 76
    #3 Please enter a number: 99
    The average of 78 76 99 is : 84
    and i copy the code...do a compile...run it again and

    How many numbers do you want to average: 8
    #1 Please enter a number: 77
    #2 Please enter a number: 8765
    #3 Please enter a number: 88
    #4 Please enter a number: 7
    #5 Please enter a number: 765
    #6 Please enter a number: 88888
    #7 Please enter a number: 765
    #8 Please enter a number: 7
    The average of 77 8765 88 7 765 88888 765 7 is : 12420

    flawlessly.... i don't know why?

  12. #12
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Probably because you had one working version, long ago; and since memory is not deleted or anything, the variables land in the same places, and integers gets a valid value by luck. Declare int *ptr = 0 and see if that works.

  13. #13
    Registered User
    Join Date
    Jul 2008
    Posts
    8
    yep, you are right. It crashes if i do int *ptr = 0. , but it still works if i undo and recompile.

    maybe one day i'll understand. thanks for your help

  14. #14
    Registered User valaris's Avatar
    Join Date
    Jun 2008
    Location
    RING 0
    Posts
    507
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    
    int allocate_memory(int *integers, int *num1);
    int average_of(int *integers, int *num1);
    int print_average(int *integers, int *num1);
    
    
    allocate_memory(int *integers, int *num1)
    {
    
                
        printf("How many numbers do you want to average: ");
        scanf("%d", num1);
        
        integers = malloc(*num1 * sizeof(int));
        
        if(integers == NULL)
          {
             printf("malloc has failed\n");
             exit(1);
          }
        
        
    }
    
    
    average_of(int *integers, int *num1)
    {
       int    i;
    
       integers[*num1] = 0;
    
       for(i = 0; i < *num1; i++)
          {
             printf("#%d Please enter a number: ", i + 1);
             scanf("%d", &integers[i]);
             integers[*num1] += integers[i];
          }
       integers[*num1] /= *num1;
    }
    
    print_average(int *integers, int *num1)
    {
       int    i;
       
       printf("The average of ");
       
       for(i = 0; i < *num1; i++)
          printf("%d ", integers[i]);
          
       printf("is : %d\n", integers[*num1]);
    }
    
    main()
    {
       int   *integers;
       int   num1;
       int *ptr; 
       integers = ptr;
      
       allocate_memory(integers, &num1);
       average_of(integers, &num1);
       print_average(integers, &num1);
       
       free(integers);
       
       fflush(stdin);
       getchar();
    }
    If you are going to change what a pointer points to, IE edit a pointer you must pass its address. so allocate_memory(&inegers, &num1);

    The function prototype would have to be changed to void allocate_memory(int **integers, int *num1);

    That is, a pointer to a pointer. Now in integers will be the memory location of your passed pointer. Dereferencing integers once with *integers will give you the memory location of what your passed in pointer points to (this is what you want to set). So something like:

    Code:
    *integers = malloc(*num1 * sizeof(int));
    Now, outside of this function, integers will point to whatever you pointed it to in the previous function.

    This is at least my understanding of pointers to pointers (which is still new!).

    GL

  15. #15
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Aside from things that are definitely depending on undefined behaviour (that is for example using uninitialized variables),

    In main:
    Code:
       integers = ptr;
    Does absolutely nothing meaningful, since both ptr and integers both have undefined values here - copying one undefined variable into another undefined variable doesn't make much sense.

    Code:
       integers[*num1] = 0;
    If we have *num1 elements in the array, this writes to the next location AFTER the memory in integers.

    Code:
    int allocate_memory(int *integers, int *num1);
    
    allocate_memory(int *integers, int *num1)
    Whilst it's legal to have a function prototype which specifies the return type (int), and then omit it when defining the function itself, it makes the code harder to read, and all for the purpose of saving 4 characters worth of typing.

    And you could make that 3 for average_of(int *integers, int *num1) by removing the * in num1 (and all the places where it is being used). You only need to pass the address of/pointer to if you intend to change the value.

    Code:
    fflush(stdin);
    In some implementations this will "clear" the input buffer, in others it is undefined behaviour. It "may do anything". Please don't use this - at best it may do what you want, but at worst it could crash the application by accessing data that isn't there (because the standard only specifies fflush() for output files). For all intents and purposes in this program it serves NO PURPOSE either. Perhaps you actually MEANT fflush(stdout), which could make some sense where it is currently located.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. funcion runs 14 times, then program crashes
    By happyclown in forum C Programming
    Replies: 9
    Last Post: 03-03-2009, 11:58 PM
  2. prog runs on 64bit - seg faults on 32bit
    By hollie in forum C Programming
    Replies: 13
    Last Post: 12-08-2006, 01:59 AM
  3. windows installer for psp runs for vs6
    By iain in forum Tech Board
    Replies: 1
    Last Post: 06-27-2004, 09:21 AM
  4. Replies: 6
    Last Post: 10-24-2002, 08:58 AM
  5. Runs fine in IDE but crashes normally!
    By Hunter2 in forum Windows Programming
    Replies: 5
    Last Post: 05-07-2002, 03:47 PM

Tags for this Thread