dynamic memory allocation C - Malloc, How to print out user inputted array.

[Travercraig]

Right I have this program that allows a user to specify how large an array should be. In the variable k. Then it should ask the user to input and scan the values of the array.

 

This part of my program works fine, however I want the program to also print out the completed array, after the scanner has inputted all of the users values of the array. Not during, which is what it currently does.

 

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

int main()
{
    int k; //variable to hold the size of the array
    int *array; //points to a memory address
    int i, j; //counters to populate and display the array
	
	printf("Enter the size of the array: "); scanf(" %i", &k); //scans in the size of the array

    array = (int *) malloc( k* sizeof(int));
	
	for (j = 1; j <= k; j++)
    {		
        for (i = 1; i <= k; i++)
	    {
			printf("Value at postion %i: ", i);	
		    scanf("%i", &array[i]);	
	    }
        printf("%i\n" ,array[j]);
	}
	
	if (!(array = (int *) malloc(k * sizeof(int))))
	    printf("Memory allocation failed!\n\n");
	
	else	
		printf("Memory allocation suceeded!\n\n");
	
    return 0;   
}

 

I get this output at the moment: (Image attached) I can see the issue but I'm not sure how to fix it. It scans the user input in, until the input is larger than k, then it prints out the first value of the array and repeats until all loops have been .

exhausted.

 

Any help would be good. Also sorry about the formatting of the code, this site messed it up.

 

[Mr_KoKa]

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

int main()
{
  int k; 
  int *array;
  int i, j; 

  printf("Enter the size of the array: "); scanf(" %i", &k); 

  array = (int *) malloc(k * sizeof(int)); // Don't you want to check if this has been allocated successfuly?

  // This loop setup has no sense for one dimensional array.
  for (j = 1; j <= k; j++) // j starts from 1 and ends on k
  {		
    for (i = 1; i <= k; i++) // Same thing as with j 
    {
      printf("Value at postion %i: ", i);	
      scanf("%i", &array[i]);	
    }
    printf("%i\n" ,array[j]); // at some point you ask for k-th index, which is out of bounds.
  }

  if (!(array = (int *) malloc(k * sizeof(int)))) // You allocate, but don't use it, also overwritting array
                                                  // which make you unable to free memory you allocated before.
    printf("Memory allocation failed!\n\n");

  else	
    printf("Memory allocation suceeded!\n\n");

  return 0;   
}

Does it should be one dimensional array or 2D? Because your loop setup looks like you want to create 2D array, but you allocating one dimensional one.

 

Another thing is that your loop starts from 1 to k and you're accessing your array this way, so when you ask for k-th element it is out out array bounds, arrays are indexed from 0 to k - 1

 

Last thing is, you're allocating memory again, after the loop, overwriting array pointer, it is memory leak. This time you do check if memory allocation has been successful but you don't check it for the first time you allocating memory.

Don't forget about freeing allocated memory.

[Enderman]

Put the printf in a separate for loop at the end of the program?

[Mira Yurizaki]

The scanner loop should be outside the for-loop and the array printer should come after the scanner at some point. Basically:;

for (i = 0; i < k; i++)
{
  printf("Value at postion %i: ", i);
  scanf("%i", &array[i]);
}

for (j = 0; j < k; j++)
{
  printf("%in" ,array[j]);
}

(also not sure why you started at 1, but I haven't used malloc in ever so if it really needs to start at 1, ignore my changes)

[wanderingfool2]

 

What @M.Yurizaki said is correct.  To expand on the thought though:

 

i = 1...k, and then using array actually write to memory that you should not write too.

 

In terms of memory management, when you use malloc, array[0] points to the beginning of the array, so array[k-1] is actually the last value you are allowed to write to.  So Yurizaki is right, you should be doing i = 0...k-1.

 

Another note about malloc, see the part below

    array = (int *) malloc( k* sizeof(int));
	
	...
	
	if (!(array = (int *) malloc(k * sizeof(int))))

 

You initially allocate memory, and then the if statement below you are creating the memory again.  The if statement should actually have been used prior to reading in the user inputs.  (If the initial allocation failed, your program would actually crash).  Also, while you are learning about memory management and stuff, may I suggest trying to avoid putting all the code in one line.  See below

array = (int*) malloc(k * sizeof(int));
if(!array) { //This is a lot more readable than your if statement

Really you should only have 1 malloc call in your code.

 

One last thing as well, remember, every time you write a malloc, be aware that free should be called.  Even though your program end, which means the memory does get released back to the operating system, it is still very good practice to free up the memory of a malloc. free(array); should be called at least before the end of the program.

 

[Mira Yurizaki]

So I was wondering how this program was running when the for loop was starting at 1 when you should be starting at 0. If this were an embedded system, the processor will happily use the data out of the array bounds, although this is for statically allocated arrays.

 

Anyway, I looked for what malloc does memory allocation wise and I got this from Stack Exchange:

Quote

Malloc allocates heap space in chunks of your system page size (typically, 4096 bytes), but it only reserved for your use exactly as much as you requested. If you write beyond your malloced memory, then you risk corrupting your heap or crashing your program with a segmentation violation.

 

Only as much memory as you requested is guaranteed to be actually yours to use. The block of memory behind the one you requested may be valid, but in use by something else (e.g. stdio library buffers) or reserved for future use (e.g. the next malloc you call).

In other words, if your array size is 5 and you start at 1 and go to 5, which 5 is out of bounds (the positions in an array size of 5 is 0 to 4), writing to 5 may still be valid but it may overwrite what's in heap in that memory location. I'd expect the OS to throw a seg fault, but I guess since you're still within your page, the OS doesn't really care.

[Travercraig]

2 hours ago, M.Yurizaki said:

The scanner loop should be outside the for-loop and the array printer should come after the scanner at some point. Basically:;

for (i = 0; i < k; i++)
{
  printf("Value at postion %i: ", i);
  scanf("%i", &array[i]);
}

for (j = 0; j < k; j++)
{
  printf("%in" ,array[j]);
}

(also not sure why you started at 1, but I haven't used malloc in ever so if it really needs to start at 1, ignore my changes)

 

2 hours ago, Mr_KoKa said:

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

int main()
{
  int k; 
  int *array;
  int i, j; 

  printf("Enter the size of the array: "); scanf(" %i", &k); 

  array = (int *) malloc(k * sizeof(int)); // Don't you want to check if this has been allocated successfuly?

  // This loop setup has no sense for one dimensional array.
  for (j = 1; j <= k; j++) // j starts from 1 and ends on k
  {		
    for (i = 1; i <= k; i++) // Same thing as with j 
    {
      printf("Value at postion %i: ", i);	
      scanf("%i", &array[i]);	
    }
    printf("%i\n" ,array[j]); // at some point you ask for k-th index, which is out of bounds.
  }

  if (!(array = (int *) malloc(k * sizeof(int)))) // You allocate, but don't use it, also overwritting array
                                                  // which make you unable to free memory you allocated before.
    printf("Memory allocation failed!\n\n");

  else	
    printf("Memory allocation suceeded!\n\n");

  return 0;   
}

Does it should be one dimensional array or 2D? Because your loop setup looks like you want to create 2D array, but you allocating one dimensional one.

 

Another thing is that your loop starts from 1 to k and you're accessing your array this way, so when you ask for k-th element it is out out array bounds, arrays are indexed from 0 to k - 1

 

Last thing is, you're allocating memory again, after the loop, overwriting array pointer, it is memory leak. This time you do check if memory allocation has been successful but you don't check it for the first time you allocating memory.

Don't forget about freeing allocated memory.

 

1 hour ago, wanderingfool2 said:

 

What @M.Yurizaki said is correct.  To expand on the thought though:

 

i = 1...k, and then using array actually write to memory that you should not write too.

 

In terms of memory management, when you use malloc, array[0] points to the beginning of the array, so array[k-1] is actually the last value you are allowed to write to.  So Yurizaki is right, you should be doing i = 0...k-1.

 

Another note about malloc, see the part below

    array = (int *) malloc( k* sizeof(int));
	
	...
	
	if (!(array = (int *) malloc(k * sizeof(int))))

 

You initially allocate memory, and then the if statement below you are creating the memory again.  The if statement should actually have been used prior to reading in the user inputs.  (If the initial allocation failed, your program would actually crash).  Also, while you are learning about memory management and stuff, may I suggest trying to avoid putting all the code in one line.  See below

array = (int*) malloc(k * sizeof(int));
if(!array) { //This is a lot more readable than your if statement

Really you should only have 1 malloc call in your code.

 

One last thing as well, remember, every time you write a malloc, be aware that free should be called.  Even though your program end, which means the memory does get released back to the operating system, it is still very good practice to free up the memory of a malloc. free(array); should be called at least before the end of the program.

 

Oh my... I did some silly things. I'm brand new to using malloc and pointers and didn't really fully understand what i was doing. @M.Yurizaki's soloution worked perfectly along with moving my if statement condition to check for memory as picked up on by @wanderingfool2 and @Mr_KoKa.  I thought I had to use a nested for loop and I wasn't sure at all about where to place the memory allocation check. Thanks a lot all. I've learnt from this which is a good thing.