Reading, echoing, and returning integers in an array

I was wanting some clarification on my code, as I am new to pointers in C. I am trying to return an array from a simple function just so I can understand exactly what it is that I'm doing.

Here is the code I have so far and, to me, it seems as if I haven't even used the pointers correctly.

#include 
#include 

int *myFunction(int size)
{
    int i;
    int *myArray = malloc(sizeof(int)*size);
    for(i=0;i<size;i++)
    {
        printf("Enter a number: ");
        scanf("%d",&myArray[i]);
    }
    for(i=0;i<size;i++)
    {
        printf("%d\n",myArray[i]);
    }
    return myArray;
}
int main()
{
    int *p;
    int size;
    printf("Enter the size: ");
    scanf("%d",&size);
    p = myFunction(size);
    return 0;
}

Someone suggested I turn my comment into an answer, so here goes.

First, I simply ran your code through astyle and added line numbers to make it easier to
reference in the text. Hopefully you can see from the output how a little bit of white-space
and consistent formatting/indentation can make your code clearer.

1  #include 
 2  #include 
 3  
 4  int *myFunction(int size)
 5  {
 6      int i;
 7      int *myArray = malloc(sizeof(int) * size);
 8  
 9      for (i = 0; i < size; i++)
10      {
11          printf("Enter a number: ");
12          scanf("%d", &myArray[i]);
13      }
14  
15      for (i = 0; i < size; i++)
16      {
17          printf("%d\n", myArray[i]);
18      }
19  
20      return myArray;
21  }
22  
23  int main()
24  {
25      int *p;
26      int size;
27      printf("Enter the size: ");
28      scanf("%d", &size);
29      p = myFunction(size);
30      return 0;
31  }

First, full marks for lines 23 and 30: many people still code as if main can be declared void main() and
fail to return an exit code. This would be just a little better for line 30.

return EXIT_SUCCESS;

As I said above, there is nothing wrong with your use of pointers (which was your primary concern). But there are
other issues with the code if this was ever to be used for anything other than exploring concepts.

Now, let's deal with dynamic memory.

MORTAL suggested zeroing all dyamically allocated memory. That can be useful (for example, if you are going
to build a c-style string in the buffer) but is not necessarily a "good thing" in all cases. However, we can
clean up line 7 a little and get that for free while we're at it. In this line, you are allocating memory to
hold an array of int - rather than just a chunk of memory.

There's a function just for that purpose, so this

int *myArray = calloc(size, sizeof(int))

very clearly expresses your need for enough memory to store size ints.

It also sets the memory block to all zeroes before returning it.

As ChrisWue mentioned, size could be a very large number such that the call to malloc or calloc could fail.
So it's a best practice to always check the returned pointer to see if it is NULL (that's how memory allocation
routines report failure). So now we have:

int *myArray = calloc(size, sizeof(int))

if (!myArray)
{
    fprintf(stderr, "Failed to allocate memory to hold your input. Exiting.\n");
    exit(EXIT_FAILURE);
}

Note that I simply bail out (indicating an error via the exit status) rather than try and deal with an out of memory
condition. In fact, there's an even chance that the fprintf won't work if your system is this low on resources
but it's worth trying to say something rather than just present the user with a commmand line prompt.

Having got over that hurdle, you should always return any dynamically allocated memory to the pool as soon as you
are finished with it. Most operating systems will release all resources when a process ends ... but what you are
looking at in your code is a classic "memory leak".

As the caller does nothing with the returned array, I would actually make a few changes.

• Change the return type of myFunction to void

• Replace line 20 with free(myArray); to release the allocated memory

• Delete line 25, and modify the call on line 29 not to save the (now non-existent) return value

Also, as a general rule, int is the wrong type for indexing arrays. First, it is signed and
myArray[-1] makes no sense. Second, and int may not be large enough to index all of memory.
The correct type to use is size_t, and indeed that's what calloc and malloc expect.

So your variables size and i should be size_t, and line 28 should change to read

scanf("%zu", &size);

where "%zu" is the correct format code for a size_t.

Your code now looks like this:

1  #include 
 2  #include 
 3  
 4  void myFunction(size_t size)
 5  {
 6      size_t i;
 7      int *myArray;
 8  
 9      myArray  = calloc(size, sizeof(int));
10      if (!myArray)
11      {
12          fprintf(stderr, "Failed to allocate memory to hold your input. Exiting.\n");
13          exit(EXIT_FAILURE);
14      }
15  
16      for (i = 0; i < size; i++)
17      {
18          printf("Enter a number: ");
19          scanf("%d", &myArray[i]);
20      }
21  
22      for (i = 0; i < size; i++)
23      {
24          printf("%d\n", myArray[i]);
25      }
26  
27      free(myArray);
28  }
29  
30  int main()
31  {
32      size_t size;
33      printf("Enter the size: ");
34      scanf("%zu", &size);
35      printf("You need to enter %zu numbers\n", size);
36      myFunction(size);
37      return EXIT_SUCCESS;
38  }

Note that I added line 35 to give a little more feedback.

There is a bigger problem with your use of scanf that goes beyond the scope of this review. Just be
aware that scanf (like gets()) is considered EVIL. Just as a simple example, try these
responses to your "Enter the size:" prom

StackExchange Code Review Q#79617, answer score: 8