How to store values in dynamic array in C

Question

I am trying to assign values to elements in dynamic array, but can't find solution. There are many videos showing how it works if value is entered by the user via scanf, but here it's not the case. I really tried to find info here and there and solve it myself so any help will be highly appreciated. Here is my code:

//Program co convert decimal number to binary and count zeros
int main()
{
    int decimalNum;
    int *binaryNum;
    int zeroCounter = 0;
    int i = 0;
    int sizeOfArray;
    int decimalNumCopied;


    printf("Please enter a number from 0 to 255: ");
    scanf("%d", &decimalNum);
    decimalNumCopied = decimalNum;

    while(decimalNum != 0)//checking number of bits;
    {
       decimalNum = decimalNum / 2;
       i++;
    }
    sizeOfArray = i;

    //Trying to allocate just enough memory 
    binaryNum = (int*)malloc(sizeOfArray * sizeof(int));


    while(decimalNumCopied != 0)
    {
        /*At next step I am trying to assign values to each element of the
        array and it doesn't work
        */
        binaryNum[i] = decimalNumCopied % 2;
        decimalNumCopied = decimalNumCopied / 2;
        i--;
    }

    for(i = 0; i <= sizeOfArray; i++)
    {
        printf("%d", binaryNum[i]);
        if(binaryNum[i]== 0){zeroCounter++;}
    }
    printf("\nThere are %d zeroes", zeroCounter);

    free(binaryNum);

    return 0;
}

And what is wrong with your code? What problems do you have with it? What *is your question*? Please take some time to [read about how to ask good questions](http://stackoverflow.com/help/how-to-ask). — Some programmer dude, Mar 13 '17 at 12:48
Also [don't cast the result of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — UnholySheep, Mar 13 '17 at 12:49
Off by one error. Valid array indices are `0` to `sizeOfArray - 1`. The body of the `while (decimalNumCopied != 0)` copies one past the end of `binaryNum` on the first iteration. — Peter, Mar 13 '17 at 13:01
@Someprogrammerdude I wasn't sure what is wrong with my code therefore asked the question. My main mistake turned out pretty obvious as writing values outside of array boundaries, but I was thinking the values are not recorded at all hence the post title. — libengazi, Mar 13 '17 at 16:04

score 4 · Answer 1 · answered Mar 13 '17 at 12:47

4

Your first assignment is outside the bounds of the array. This gives you undefined behavior.

Remember that arrays are indexed from zero, so if i is the length of the array it is not a valid index (it's one beyond the last valid index).

answered Mar 13 '17 at 12:47

unwind

391,730
64
469
606

Thank you very much for your answer. I was somehow sure I mixed up something with the pointer and missed such an obvious mistake. – libengazi Mar 13 '17 at 15:33

score 3 · Answer 2 · answered Mar 13 '17 at 13:16

There are two errors here.

First, when you start assigning values to the array i is equal to the number of elements in the array. Since arrays in C have indexes from 0 to n-1, where n is the length, you're writing one element past the end of the array. Doing so invokes undefined behavior, which in this case (lucky for you) manifests in a crash.

You need to decrement i once before entering the loop to start writing at the proper offset.

i--;
while(decimalNumCopied != 0)
{
    ...

The second issue is with the printing. Your for loop starts at 0 (as it should), but stops when i <= sizeOfArray is no longer true. So on the last iteration i is equal to sizeOfArray, so read one element past the end of the array. Again, this invokes undefined behavior.

Change the condition to < to prevent this:

for(i = 0; i < sizeOfArray; i++)

score 0 · Answer 3 · answered Mar 13 '17 at 14:21

I felt this was a useful program so incorporating the answer by dbush and others, here it is fixed, valgrind checks good, with some user input error handling, some stylistic changes, and alternatively also accepts input from the command line.

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

int main(int argc, char**argv) {
    int decimalNum;
    int *binaryNum;
    int zeroCounter = 0;
    int i = 0;
    int sizeOfArray;
    int decimalNumCopied;

    if (argc == 2) {
        decimalNum = atoi(argv[1]);
    }
    if(argc == 1 || decimalNum < 0 || decimalNum > 255) {
        printf("Please enter a number from 0 to 255: ");
        scanf("%d", &decimalNum);
        while (decimalNum < 0 || decimalNum > 255) {
            printf("Program is expecting an int from 0 to 255 inclusive\n"
                "Please enter a number from 0 to 255: ");
            scanf("%d", &decimalNum);
        }
    }
    decimalNumCopied = decimalNum;

    while (decimalNum != 0) {
        decimalNum /= 2;
        i++;
    }
    sizeOfArray = i;

    binaryNum = malloc(sizeOfArray * sizeof(int));

    i--;
    while (decimalNumCopied != 0) {
        binaryNum[i] = decimalNumCopied % 2;
        decimalNumCopied /= 2;
        i--;
    }

    for (i = 0; i < sizeOfArray; i++) {
        printf("%d", binaryNum[i]);
        if (binaryNum[i] == 0) { zeroCounter++; }
    }
    printf("\nThere are %d zeroes\n", zeroCounter);

    free(binaryNum);

    return 0;
}

I suppose it's a lame question, but would you explain why malloc shouldn't be typecasted as integer(the type of variable binaryNum) in this case? — libengazi, Mar 13 '17 at 15:48
No need to answer the above question - I read this topic: http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc?noredirect=1&lq=1 — libengazi, Mar 13 '17 at 16:12

How to store values in dynamic array in C

3 Answers3