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.

Quote:

---

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

---

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.

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.

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

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.

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. http://cboard.cprogramming.com/showthread.php?t=105380

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.

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.

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 :(

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

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?

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.

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

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

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

valaris : thanks, but your suggestions didn't work for me.

i don't know how to make this damn thing run using a pointer to a pointer, so i'm just gonna stick with returning the pointer at the end of the function (this now makes alot of sense to me).
I've also updated a few coding faux pas (except 2):

some serious

Code:

---

integers = malloc(*num1 * sizeof(int));  to  integers = malloc((*num1 + 1) * sizeof(int));

---

Code:

---

int  *integers = 0; // undefined behavior bad
int  num1 = 0;

---

some not so serious

Code:

---

main() to int main(void)

---

Code:

---

return 0;

---

Code:

---

int *allocate_memory(int *integers, int *num1); // not just the prototypes
int average_of(int *integers, int *num1);
int print_average(int *integers, int *num1);

---

some just plain stupid (in an attempt to understand)

Code:

---

integers = ptr;

---

But.....

1)fflush(stdin) must stay, because the getchar() doesn't work if its not there...see for yourself, it doesn't wait for a character return.
matsp: fflush(stdout) does not make sense to me. It flushes the standard output, after those scanfs i have to flush the standard input to do a getchar().

2)Between copying a value in to a function (int num1) vs copying a pointer to a value in to a function (int *num1). I don't know whether one or the other is more efficient, or if they are equal. I know i'm not changing the value of num1 (except in the allocate_memory function), but do i really need to change all of the other int *num1's to int num1's... ?

heres my code:

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);


int *allocate_memory(int *integers, int *num1)
{

           
    printf("How many numbers do you want to average: ");
    scanf("%d", num1);
   
    integers = malloc((*num1 + 1) * sizeof(int));
   
    if(integers == NULL)
      {
        printf("malloc has failed\n");
        exit(1);
      }
   
    return integers;
}


int 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;
}

int 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]);
}

int main(void)
{
  int  *integers = 0;
  int  num1 = 0;
   
 
 
  integers = allocate_memory(integers, &num1);
  average_of(integers, &num1);
  print_average(integers, &num1);
 
  free(integers);
 
  fflush(stdin);
  getchar();
 
  return 0;
}

---

Thanks again, to everyone who has posted so far.

Quote:

---

Originally Posted by polymatrix

But.....

1)fflush(stdin) must stay, because the getchar() doesn't work if its not there...see for yourself, it doesn't wait for a character return.

---

There is no "see for yourself" with undefined behavior. matsp can stick it in his code and it may crash the program, or it may delete the file, or it may start playing minesweeper on his computer. The idea of something being "undefined" means that the C standard allows anything to happen, which means your compiler and system may work the way you intended (ie. discard chars you don't want), but other compilers and systems may do something not-so-nice and unexpected (ie. crash or even mutilate or lose valid input). Either way, no matter what it does, the C standard says it's ok because calling fflush() on an input stream is not defined.

Thanks Macgyver. What should i use to flush the standard input?

And how to solve it is in the FAQ How do I ... (Level 2):
http://faq.cprogramming.com/cgi-bin/...&id=1043284392

--
Mats

Quote:

---

Originally Posted by tabstop

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.

---

of course you could always pass a pointer to the pointer as so -

Code:

---

void foo(int** integers , int num){
  *integers = (int*)malloc(num * sizeof(int));
 
  return;
  }
 
void main(void){ // love you sweety
 
  int* integers;
  int num = 5;
 
  printf("integers* = %d\n" , integers);
 
  foo(&integers , num);
 
  // integers now points to an array of integers
  printf("integers* = %d\n" , integers);
 
  return;
  }

---

This is weird to me...but its pretty cool. I was not familiar with the ** operator.
So, ** essentially means: a pointer to a pointer? Valaris, your previous post makes more sense to me now. You gave me the answer, thank you.

Abachler: thanks... your code helped me see what i was doing wrong. I wasn't calling allocate_memory(&integers, &num1)... i was calling ( integers, &num1). I thought &integers would be redundant syntax (the address of the address of integers), its not. I'm kinda sorta starting to get it. **integers is a pointer to *integers address, so in the function the address of *integers is alterable...right?

Macgyver and matsp: I only hope, one day i'll understand the language like you too do. From what i can gather the new version of fflush(stdin) is:

Code:

---

int ch;
while ((ch = getchar()) != '\n' && ch != EOF);

---

1)This creates an int variable called ch
2)executes an infinite do nothing while loop if the condition ((ch = getchar()) != '\n' && ch != EOF); is met.
3)((ch = getchar()) != '\n' && ch != EOF) means ch equals the return value of getchar . if this is not equal to a newline character and ch is not equal to EOF do nothing and evaluate the condition again.

Is this right? because i don't get it.
It works though.... just replaced fflush(stdin) with
while ((ch = getchar()) != '\n' && ch != EOF); and it does the same thing.
for me though, its more undefined than fflush(stdin) is to the compiler. I would really appreciate an explanation .

here's my new code:

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);


int allocate_memory(int **integers, int *num1)
{

           
    printf("How many numbers do you want to average: ");
    scanf("%d", num1);
   
    *integers = malloc((*num1 + 1) * sizeof(int));
   
    if(integers == NULL)
      {
        printf("malloc has failed\n");
        exit(1);
      }
   
}


int 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;
}

int 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]);
}

int main(void)
{
  int  *integers = 0;
  int  num1 = 0;
  int  ch;
 
 
  allocate_memory(&integers, &num1);
  average_of(integers, &num1);
  print_average(integers, &num1);
 
  free(integers);
 
  while ((ch = getchar()) != '\n' && ch != EOF);
  getchar();
 
  return 0;
}

---

Is this better?
Is this standard C?
Thanks everybody.

Definitely better.
Is it standard - well, "gcc -Wall -ansi -pedantic -O2" gives no errors/warnings indicating non-standardness, so that's a good sign. You do have some warnings, but that's commented on below.

Your understanding of the "remove input" (or fflush(stdin)) is correct - it reads one character at a time until it reaches either newline or end of file.

In allocate_memory(): Whilst this line is syntactically correct, it is not what you want:

Code:

---

    if(integers == NULL)

---

You want to check if the value returned from malloc is NULL, not if the pointer you passed in from main is NULL, so check *integers [although, if you want to be really pedantic, yes, you should check that TOO - but that should be done BEFORE the call to malloc, and if we are that pedantic, you should also check if num1 is NULL].

In average_of(): You do not need to pass num1 as a pointer. Same with print_average().

I personally would not use the last element of the array to store the average, but instead return a value from the average_of function, and pass that value to the print_average() function.

All of your functions have an integer return type, but besides main, all functions have no return statement. You may want to make those functions have a void return type.

--
Mats