Click here to Skip to main content
15,885,985 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
So, i'm trying to make a program, that allocates enough space for 5 integers using malloc and a pointer to act as the array, and through a function, i'm doubling the size of the "array" using realloc, inserting the square of every number of the first 5, into the 5 next spaces. It's hard to explain so here is an example:

The original array before "realloc"
arr[0]=1
arr[1]=2
arr[2]=3
arr[3]=4
arr[4]=5

After realloc and calculations:

arr[5]=1
arr[6]=2
arr[7]=9
arr[8]=16
arr[9]=25

So basically, the content of array[x] gets squared and inserted in array[x+5]

The problem is that my program works fine, I get the above array as the result, but as soon as it shows the results, it crashes and I can't figure out why. Here's my code:

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


int * double_old(int *old, int old_size, int new_size);

int main(void)
{
	int *old;
	int old_size = 5;
	int new_size;
	
	old=(int*) malloc(old_size*sizeof(int));
	
	
	printf("Enter 5 integers: \n\n\n");
	
	for(old_size = 0; old_size < 5; old_size++)
		{
			scanf("%d", &old[old_size]);	
		}
		
	printf("The original array is: \n");
	
	for(old_size = 0; old_size < 5; old_size++)
		{
			printf("old[%d] = %d\n", old_size, old[old_size]);
		}
		
	printf("\n\n\n");	
	double_old(old, old_size, new_size); 
										
										
	free(old);  
	
}

int * double_old(int *old, int old_size, int new_size)
{		
	int i=5, a=0;
	new_size = old_size*3;
	old = realloc(old, new_size);
	
	printf("The results based on the 5 previous numbers: \n");

	for(;old_size<10;old_size++)
	{
		old[old_size] = old[old_size-i]* old[old_size-i];				
		printf("old[%d] = %d\n", old_size, old[old_size]);
	}
}


What I have tried:

I've been trying for the last 2 days to understand what I'm doing wrong, but haven't found any solutions yet. Maybe it has something to do with malloc since I don't fully understand how it works as I'm kind of new to C and in programming.
Posted
Updated 18-Jan-17 0:15am
v3
Comments
Jochen Arndt 18-Jan-17 5:50am    
You already got a solution from Richard.

But there should be a compiler warning like
"no return statement in function returning non-void"

If not, increase the warning level of the compiler.
With GCC use -Wall. With VisualStudio use /Wall (set it in the project settings).

Having such warnings and fixing the code until all warnings are gone will save you a lot of time.
Member 12955853 18-Jan-17 6:00am    
Mr Richard's solution sadly didn't work. However ppolymorphe's answer fixed it for me. The problem was that when I re-allocated I put "old = realloc(old, new_size);"
wth "new_size" being 10. I thought this meant that the re-allocation reserves space for 10 integers but forgot that I had to put:
old = realloc(old,new_size*sizeof(int));


I'm using Dev C++ btw, that uses Gcc. Yes I know it sucks but that's the one they told us to use in university
Jochen Arndt 18-Jan-17 6:05am    
Sorry, I did not checked the answer and thought that it fixed the problem.

But you should still set the warning level.

I don't know Dev C++ but it should have an configuration option.
A quick search assumes that it is at Tools - Compiler Options - Warnings tab.
Richard MacCutchan 18-Jan-17 6:39am    
Yes, sorry I missed that bit.

You forgot to return the updated pointer from the double_old function. The code should be:
C++
	old = double_old(old, old_size, new_size); 								
										
	free(old);  
	
}

int * double_old(int *old, int old_size, int new_size)
{		
	int i=5, a=0;
	new_size = old_size*3; // why times 3?
	old = realloc(old, new_size);
	
	printf("The results based on the 5 previous numbers: \n");

	for(;old_size<10;old_size++)
	{
		old[old_size] = old[old_size-i]* old[old_size-i];				
		printf("old[%d] = %d\n", old_size, old[old_size]);
	}
    return old; // return the realloc'ed array pointer
}
 
Share this answer
 
Comments
Member 12955853 18-Jan-17 5:43am    
That's weird. I fixed it like you said, but the program still crashes as soon as it shows the results.
As for the "old_size*3", it was supposed to be "old_size*2" but I changed it to test something and forgot to change it back. Here's my code after the fix:

old = double_old(old, old_size, new_size);


free(old);
}

int * double_old(int *old, int old_size, int new_size)
{
int i=5;
new_size = old_size*2;
old = realloc(old, new_size);

printf("The results based on the 5 previous numbers: \n");

for(;old_size<new_size;old_size++)
{
old[old_size] = old[old_size-i]* old[old_size-i];
printf("old[%d] = %d\n", old_size, old[old_size]);
}
return old;
}
When you allocate, it is 5 integers
C++
old=(int*) malloc(old_size*sizeof(int));

When you reallocate, it is 15 bytes, no matter what is the size of an integer.
C++
new_size = old_size*3;
old = realloc(old, new_size);
 
Share this answer
 
Comments
Member 12955853 18-Jan-17 5:53am    
Oh, I think I know what you're saying. I changed:
old = realloc(old, new_size); to
old = realloc(old, new_size*sizeof(int));

and now it won't crash. Thanks.
nv3 18-Jan-17 8:04am    
Well spotted!
Patrice T 18-Jan-17 8:10am    
Thank you
Don't mess with old_size. Try
C
#include <stdio.h>
#include <stdlib.h>


int * double_old(int *old, int old_size);

int main(void)
{
  int *old;
  const int old_size = 5;
  int i;

  old=(int*) malloc(old_size*sizeof(int));


  printf("Enter 5 integers: \n\n\n");

  for(i = 0; i< old_size; i++)
    {
      scanf("%d", &old[i]);
    }

  printf("The original array is: \n");

  for(i = 0; i < old_size; i++)
    {
      printf("old[%d] = %d\n", i, old[i]);
    }

  printf("\n\n\n");
  old = double_old(old, old_size);

  free(old);
}

int * double_old(int *old, int old_size)
{
  int i;
  int new_size = old_size * 2;
  printf("old = %p\n", old);
  old = realloc(old, new_size*sizeof(int));

  printf("The results based on the 5 previous numbers: \n");
  for(i = old_size;i<new_size;i++)
  {
    old[i] = old[i-old_size]* old[i-old_size];
    printf("old[%d] = %d\n", i, old[i]);
  }
  return old;
}
 
Share this answer
 
Richard has already pointed out the bug in your program. There is nothing to add to that.

As you are new to programming you should try to pick up the right habits from the very beginning. Therefore, I want to point out a couple of things to you could do better in this small example and which will make things clearer for you and for the reader of your programs.

1. Naming
Why do you call the pointer to your array old? Always use names that tell something about what's stored in the variable, structure or array. In this case we don't know what exactly is being stored here, so why don't we call it values (note the use of the plural!). And your variable old_size is in fact to hold the current size of the array, so why don't we simply call it size.

Now to your function. The main purpose of the function is to calculate the squares of all values in the input array. So it should be called something like CalcSquares.

2. Be consistent in using loop indices
In your main you allocate the array and should set the size variable straight away. We know that the array has this size, so we store it immediately:
C++
int size;
...
size = 5;
values = (int*) malloc (size * sizeof(int));

Don't abuse the size variable as loop index. Use a separate variable for that and refer to the give size of the array always and only by your size variable:
C++
int i:
...
for (i = 0; i < size; ++)
{
   ...
}

If you form your loops all like this it will be very easy to pinpoint any mistakes.

3. Understand parameter passing
Probably the biggest flaw in you program is the new_size parameter to the function old_double. It is passed by value. But you later use it as if you would expect it to be passed by reference. "By value" means, the value you set inside old_double will never be passed back to the caller! If you would like the caller to know what the new size of the array is, use passing "by reference" or in plain old C that is passing a pointer:

C++
int* CalcSquares (int* pValues, int size, int* pNewSize)
{
  int i, newSize;

  // allocate the array extension
  newSize = size * 2;
  pValues = realloc (pValues, newSize * sizeof(int));
  if (pValues == 0)
  {
    *pNewSize = 0;
    return 0;
  }

  // calculate the squares
  for (i = size; i < newSize; ++i)
    pValues [i] = pValue[i-size] * pValue[i-size];

  // return the result
  *pNewSize = newSize;
  return pValues;
}


4. Avoid printing in your worker functions
You want your function to perform one specific task, in that case calculating the squares. Don't intermix those things with printing. If you want to use your function later in another context you probably would not like the printing inside the function, but do it somewhere outside by the calling code.

5. Don't make allocation of memory to complicated
I assume you are doing this exercise to learn about realloc and that is fine. But in "normal" code you would try to keep the allocation of memory and the interface of your function as simple as possible.
 
Share this answer
 
v2
Comments
Richard MacCutchan 18-Jan-17 6:41am    
Like me you missed the fact that the realloc call was incorrect. See ppolymorphe's answer.
nv3 18-Jan-17 8:04am    
Oops, you are so right, Richard. Corrected that in my answer.
Member 12955853 18-Jan-17 8:11am    
@nv3 Thank you for all the tips, they really helped me understand the concept of functions. As for the names you mentioned(e.g old, old_size), we were given this by the professor and I decided to go with them, I wouldn't normally use these names as they don't make much sense but I don't want the professor to be whining about how I didn't use the variable names I was given bla bla bla... You know what I mean.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

  Print Answers RSS
Top Experts
Last 24hrsThis month


CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900