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

polymatrix · 07-20-2008, 07:31 PM

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.

Quote:

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

MacGyver · 07-20-2008, 07:41 PM

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: Segfaults - again

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

tabstop · 07-20-2008, 07:44 PM

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.

rags_to_riches · 07-20-2008, 07:47 PM

Off-topic: tabstop, I had to steal that for my sig. With proper attribution, of course

Dino · 07-20-2008, 08:37 PM

Quote:

Originally Posted by rags_to_riches

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.

polymatrix · 07-20-2008, 09:30 PM

Quote:

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. Segfaults - again

MacGyver · 07-20-2008, 09:39 PM

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.

tabstop · 07-20-2008, 09:50 PM

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.

polymatrix · 07-20-2008, 11:21 PM

MacGyver: Is this what you meant by

Quote:

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

tabstop · 07-20-2008, 11:34 PM

Quote:

Originally Posted by polymatrix

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"?

polymatrix · 07-20-2008, 11:46 PM

I compiled it on Dev-C++ 4.9.9.2
It runs flawlessly

Quote:

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

Quote:

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?

tabstop · 07-20-2008, 11:50 PM

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.

polymatrix · 07-20-2008, 11:58 PM

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

valaris · 07-21-2008, 08:44 AM

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

matsp · 07-21-2008, 09:11 AM

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

07-20-2008, 07:41 PM	#2
MacGyver Deathray Engineer Join Date: Mar 2007 Posts: 3,211	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: Segfaults - again I just explained the same concept today about why memory allocation in this manner fails. __________________
MacGyver is offline

07-20-2008, 09:39 PM	#7
MacGyver Deathray Engineer Join Date: Mar 2007 Posts: 3,211	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. __________________
MacGyver is offline

07-20-2008, 09:50 PM	#8
tabstop and the Hat of Guessing Join Date: Nov 2007 Posts: 8,740	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.
tabstop is offline

07-20-2008, 11:50 PM	#12
tabstop and the Hat of Guessing Join Date: Nov 2007 Posts: 8,740	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.
tabstop is offline

07-20-2008, 11:58 PM	#13
polymatrix 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
polymatrix is offline

07-20-2008, 07:44 PM	#3
tabstop and the Hat of Guessing Join Date: Nov 2007 Posts: 8,740	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.
tabstop is offline

07-20-2008, 07:47 PM	#4
rags_to_riches and the Hat of Ass Join Date: Dec 2007 Posts: 730	Off-topic: tabstop, I had to steal that for my sig. With proper attribution, of course
rags_to_riches is offline

07-21-2008, 09:11 AM	#15
matsp Kernel hacker Join Date: Jul 2007 Location: Farncombe, Surrey, England Posts: 15,686	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.
matsp is offline

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode

Similar Threads
Thread	Thread Starter	Forum	Replies	Last Post
funcion runs 14 times, then program crashes	happyclown	C Programming	9	03-03-2009 11:58 PM
prog runs on 64bit - seg faults on 32bit	hollie	C Programming	13	12-08-2006 01:59 AM
windows installer for psp runs for vs6	iain	Tech Board	1	06-27-2004 09:21 AM
What is wrong with this simple program? runs in the background and takes all of cpu	Shadow12345	Windows Programming	6	10-24-2002 08:58 AM
Runs fine in IDE but crashes normally!	Hunter2	Windows Programming	5	05-07-2002 03:47 PM