Is this code doing what I want it to do?

Question

I want to create an integer pointer p, allocate memory for a 10-element array, and then fill each element with the value of 5. Here's my code:

//Allocate memory for a 10-element integer array.
int array[10];
int *p = (int *)malloc( sizeof(array) );

//Fill each element with the value of 5.
int i = 0;
printf("Size of array: %d\n", sizeof(array));
while (i < sizeof(array)){
    *p = 5;
             printf("Current value of array: %p\n", *p);
    *p += sizeof(int);
    i += sizeof(int);
}

I've added some print statements around this code, but I'm not sure if it's actually filling each element with the value of 5.

So, is my code working correctly? Thanks for your time.

Answer 1

First:

*p += sizeof(int);

This takes the contents of what p points to and adds the size of an integer to it. That doesn't make much sense. What you probably want is just:

p++;

This makes p point to the next object.

But the problem is that p contains your only copy of the pointer to the first object. So if you change its value, you won't be able to access the memory anymore because you won't have a pointer to it. (So you should save a copy of the original value returned from malloc somewhere. If nothing else, you'll eventually need it to pass to free.)

while (i < sizeof(array)){

This doesn't make sense. You don't want to loop a number of times equal to the number of bytes the array occupies.

Lastly, you don't need the array for anything. Just remove it and use:

int *p = malloc(10 * sizeof(int));

For C, don't cast the return value of malloc. It's not needed and can mask other problems such as failing to include the correct headers. For the while loop, just keep track of the number of elements in a separate variable.

Answer 2

Here's a more idiomatic way of doing things:

/* Just allocate the array into your pointer */
int arraySize = 10;
int *p = malloc(sizeof(int) * arraySize);

printf("Size of array: %d\n", arraySize);

/* Use a for loop to iterate over the array */
int i;
for (i = 0; i < arraySize; ++i)
{
    p[i] = 5;
    printf("Value of index %d in the array: %d\n", i, p[i]);
}

Note that you need to keep track of your array size separately, either in a variable (as I have done) or a macro (#define statement) or just with the integer literal. Using the integer literal is error-prone, however, because if you need to change the array size later, you need to change more lines of code.