0

I am trying to write a simple program in C that stores elements in a pointer. I am getting junk values when I print the elements of the array.

Here is the code:

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

#define MAX 5

int main(int argc, char const *argv[]) {

    int* a = (int*)malloc(MAX * sizeof(int));
    int* b = NULL;
    int* const maxAddress = a + MAX;

    if(a != NULL) {
        for(int index = 0; b = a, b < maxAddress; index++) {
            *(a + index) = index;
            printf("Value: %d, Index: %d\n", *(a + index), index);
            a++;
        } 
    }   

    printf("\n\n");

    for(int index = 0; index < MAX; index++) // Getting junk values here
        printf("Index: %d, Value: %d, Address: %p\n", index, *(a + index), a + index);

    return 0;
}
Mad Physicist
  • 107,652
  • 25
  • 181
  • 264
KylaM26
  • 17
  • 5
  • You may want to format your code by indenting it by four spaces, and to add a language tag, such as C. Use the edit button beneath your question, hilight your code, and press the `{}` toolbar button. – Mad Physicist Nov 07 '18 at 16:55
  • 1
    I fixed it this time, but it does really help to use the code format. Good job delivering a complete code example. – erik258 Nov 07 '18 at 16:56
  • I am not sure I understand why you say "It seems to work" when the remainder of the question shows quite the opposite. Overall nicely written question though. +1 – Mad Physicist Nov 07 '18 at 16:57
  • What is `b` for? Why is `maxAddress` ? Most important question, what does `a++` do to the pointer of the beginning of your array ? – erik258 Nov 07 '18 at 17:00
  • @DanFarrell, You are correct I don't need b, max address is just a variable that stores the the last address in the array. a++ increments as long as the a < maxAddress is true. – KylaM26 Nov 07 '18 at 17:10
  • 2
    [don't cast the result of `malloc` in C](https://stackoverflow.com/q/605845/995714) – phuclv Nov 07 '18 at 17:17

3 Answers3

0

You have two independent increments going on. In the for loop you do index++, while in the end of the loop you increment a++. That means your first write is to a[0], your next is to a[1 + 1], etc. Basically, you are skipping every other element.

To avoid doing this increment a or index, but not both. Since you want the values of index to progress, I would recommend removing the line a++.

The variable b is not used except in the beginning of the loop, so the condition b = a, b < maxAddress seems pointless. It could be written as a < maxAddress. If I had so guess, b was being used in a fuller version of the code to retain the previous value of a for the next iteration.

Mad Physicist
  • 107,652
  • 25
  • 181
  • 264
0

Here is the corrected code: b should be initialized before the the loop. the for loop was incorrect you should increment b not a in the loop

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

#define MAX 5

int main(int argc, char const *argv[]) {

    int* a = (int*)malloc(MAX * sizeof(int));
    int* b = a;
    int* const maxAddress = a + MAX;

    if (a != NULL) {
        for (int index = 0; b < maxAddress; index++) {
            *(a + index) = index;
            printf("Value: %d, Index: %d\n", *(a + index), index);
            b++;
        }
    }

    printf("\n\n");

    for (int index = 0; index < MAX; index++) // Getting junk values here
        printf("Index: %d, Value: %d, Address: %p\n", index, *(a + index), a + index * sizeof(int));

    return 0;
}
Paul Baxter
  • 1,054
  • 12
  • 22
  • 1
    Thanks for your response Paul :) – KylaM26 Nov 07 '18 at 17:21
  • 2
    Note here that `a` isn't incremented. If you increment `a`, you lose track of where the original array started. That's where the weird values came from. – erik258 Nov 07 '18 at 18:10
  • 1
    Note that even though this code does check for the null-ness of `a` when it is being populated, that is not checked for when the contents of `a` are printed in the next loop. This is best handled by checking `if(a == NULL) { /* report alllocation error and exit */ }` rather than `if(a != NULL) { /* do stuff */ } ` – Govind Parmar Nov 07 '18 at 19:31
0

The syntax *(a + index) is equivalent to a[index]; the latter indicates your intent with the code more clearly.

If we break down what you're doing with the lines index++ (in your for loop declaration) and a++ in the body of your loop, you will observe that you're accessing memory you didn't actually allocate for a:

Loop Iteration #0 (a = allocated address index = 0):

*(a + index) accesses a[0]
a[0]   a[1]   a[2]    a[3]    a[4]
 0      ?      ?       ?        ?

Loop Iteration #1 (a = alloc+1 index = 1):

*(a + index) accesses a[2]
a[0]   a[1]   a[2]    a[3]    a[4]
 0      ?      1       ?       ?

You begin to write past the array at iteration #3:

*(a + index) accesses a[6]
a[0]   a[1]   a[2]    a[3]    a[4]     | MEMORY NOT BELONGING TO a
 0      ?      1       ?       ?                   3?

Also, after your initial for loop where you incremented the pointer a to the end of your allocated amount, you try to access it again without resetting it to its initial value, so the values you access are not even the ones you (thought you had) assigned.

This is especially bad for memory allocated with malloc since you usually later free it, but you can't do that without a pointer to the beginning of your allocated memory block.

Solution:

Do not increment a in your initial for loop and use the correct truth condition:

for(int index = 0; (a + index) < maxAddress; index++) 
// Also valid: ...; index < MAX; ...
{
        *(a + index) = index;
        printf("Value: %d, Index: %d\n", *(a + index), index);
} 
Govind Parmar
  • 20,656
  • 7
  • 53
  • 85