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 <stdio.h>
2 #include <stdlib.h>
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
int
s.
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 <stdio.h>
2 #include <stdlib.h>
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:" prompt (leave out the quotes)
- "a very long string"
- "10 followed by some text"
- "-1"