0

Problem:

I'm rather sure my issue here is a typical misunderstanding of what's happening with pointers. I'll post what's happening in memory below, because my main goal here is to learn how to debug properly, so this is probably a long post about an otherwise trivial problem.

Here's the code, first:

/*Generate array with 500 to 1000 elements*/
/*In the calling function, I create something like 'int* x;' and pass '&x'*/

void gen_array(int** arr){
    int size = rand() % (1001 - 500) + 500;
    int i;

    *arr = (int*) malloc(size*sizeof(int));

    if (*arr == NULL){
        printf("Allocation failed\n");
        exit(1); //I know this is probably bad form
    }

    //Fill with some random numbers from 1 to 1000
    for (i = 0; i < size; ++i)
        *arr[i] = rand() % 1001;
}

It breaks after the first iteration in the for loop (that is, when i==1). I'm not sure if I'm allocating wrong with malloc, or assigning wrong in the loop.

Calling like this:

int* x = NULL; //Tried without '= NULL' as well
gen_array(&x);

The specific error:

Unhandled exception at 0x00265E55 in test.exe: 0xC0000005: Access violation writing location 0xCCCCCCCC.

This is after a first-chance exception at the exact same place, but they are generated at the same line of code (and on the same iteration during execution).

My attempt at analyzing:

If it helps, here's the (relevant) step-by-step from debugging on one attempt (I'm compiling in Visual Studio). I'd like to know how to use this info to help me debug in the future.

  • At the start of the calling function, x = 0xcccccccc (0xcccccccc=-842150451 seems to be what the compiler uses to fill uninitialized values). After set to NULL, it's equal to 0x00000000 of course.
  • Calling gen_array(int** arr), since I'm passing a pointer to a pointer, the local 'double pointer' is an address pointing to the NULL pointer: arr=0x0018facc {0x00000000 {???}}. (This is showing the values of what's being pointed to, since the NULL pointer is pointing to nothing, there's ???)
  • Call to malloc: now arr=0x0018facc {0x00e97048 {-842150451}}, so *arr is pointing to an uninitialized value
  • First iteration (i==0), the number generated is 588, so now arr=0x0018facc {0x00e97048 {588}} (that is, the value that *arr is pointing at is 588)

The next iteration crashes after i is increased to 1. Since it's an access violation, I would typically guess that arr+1 isn't expecting to be written to. But the violation location is at 0xCCCCCCCC, the value that this compiler uses for uninitialized data. Is this because arr+1 is technically uninitialized?

Edit: as @GrzegorzSzpetkowski advised, I tried changing *arr[i] to (*arr)[i]. It stopped breaking! but, no new random values are being created/placed into the array (which is why I'm concerned that an array isn't being created at all).

Edit 2: To @EOF's suggestion, I changed the function to return the size so I can call with something like int length = gen_array(&x);

I know I can also try returning an int* from the function, but I'd like to know what's going on specifically here in the case of passing the pointer from a calling function to be initialized.

galois
  • 825
  • 2
  • 11
  • 31
  • [Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). – Sourav Ghosh May 18 '16 at 19:23
  • In Visual Studio, since it's compiling as C++, I get an error (the build fails) when I don't cast: `error C2440: '=' : cannot convert from 'void *' to 'int *'` – galois May 18 '16 at 19:24
  • Have you included `stdlib`? – Eugene Sh. May 18 '16 at 19:25
  • 2
    `*arr[i]` -> `(*arr)[i]` because postfix has higher precedence. The former lhs does not make sense in your context. – Grzegorz Szpetkowski May 18 '16 at 19:26
  • I have included `stdlib`. – galois May 18 '16 at 19:26
  • Anyway, I believe you can force VS to compile it as C. And @GrzegorzSzpetkowski gave you the answer. – Eugene Sh. May 18 '16 at 19:27
  • @GrzegorzSzpetkowski, actually I should have mentioned that I tried that, and the function doesn't break, but when it returns, the array only has one value. – galois May 18 '16 at 19:28
  • Edited main post with his advice – galois May 18 '16 at 19:31
  • 1
    Your function is nonsense anyway. If the size of the allocation you create is random, and you don't have any way of informing the caller about it, the allocation *cannot* be safely accessed once the function returns. – EOF May 18 '16 at 19:33
  • @jaska How do you know it has one value? You don't know what was allocated and the items are random. – Eugene Sh. May 18 '16 at 19:34
  • I'm guessing now that it's the wrong way to do it, but just by: `#define length(x) sizeof(x) / sizeof(x[0])`. It works fine for another (statically allocated) array in the calling function. – galois May 18 '16 at 19:37
  • It can work only for static arrays. – Eugene Sh. May 18 '16 at 19:38
  • @EOF - okay, but, could you explain why not? How would it be different than doing the same thing in the calling function? – galois May 18 '16 at 19:40
  • In calling function you would have the `size` variable accessible... Just make it a return value, so the caller knows how large is the array. – Eugene Sh. May 18 '16 at 19:41
  • @jaska: How does the caller of this function know *how many elements it is allowed to access*? C doesn't have bounds-checking, if the caller of this function tries to access an element of the array beyond the end of the allocation, the behavior is *undefined*. – EOF May 18 '16 at 19:43
  • Gotcha - I see. I updated the post with your suggestion: I changed the function to return the size. – galois May 18 '16 at 19:54

2 Answers2

2

Please check operator precence of * and [] http://en.cppreference.com/w/c/language/operator_precedence

*arr[i] = rand() % 1001;

means

*(arr[i]) = rand() % 1001;

but you want

(*arr)[i] = rand() % 1001;
mrtnlrsn
  • 1,105
  • 11
  • 19
0

Only one error is immediately obvious to me: when you initialize the array, you need (*arr)[i] =, not *arr[i] =.

Lee Daniel Crocker
  • 12,927
  • 3
  • 29
  • 55