-4

Hi I have recently started learning c pointers and can't understand why am i getting a different pointer value than expected. Here is the function

int test(void){
    int i,j; 
    int **p = (int **)malloc(2 * sizeof(int *)); 
    p[0] = (int *)malloc(2 * sizeof(int)); 
    printf("p[0] = %d\n",p[0]);
    p[1] = p[0]; 
    printf("p[1] = %d\n",p[1]);
    for(i = 0; i < 2; i++){ 
            for(j = 0; j < 2; j++) {
                    p[i][j] = i + j;
                    printf("p[%d][%d] = %d\n",i,j,p[i][j]);
            }
    } 
    printf("result p[0][0] = %d\n",p[0][0]); 
 return 0;
}

I am expecting p[0][0] = 0 as printed from the for loop, however it is equal to 1 and the value seems to be coming from p[1][0], what am i missing here?

printf output:

p[0] = 1224416
p[1] = 1224416
p[0][0] = 0
p[0][1] = 1
p[1][0] = 1
p[1][1] = 2
result p[0][0] = 1
anonymous007
  • 319
  • 1
  • 3
  • 12
  • 4
    You're missing the fact you've got this in your code: `p[1] = p[0];` – Chris Turner Aug 18 '17 at 13:36
  • 3
    `p[1] = p[0];` <- this assigns pointers ... –  Aug 18 '17 at 13:36
  • Aside: please print a pointer like this: `printf("p[0] = %p\n", (void*)p[0]);` – Weather Vane Aug 18 '17 at 13:37
  • 1
    That is not your actual code. The newline is incorrect `/n` ==> `\n`. – Weather Vane Aug 18 '17 at 13:38
  • @Ron depends. In the question's code: yes. There's no need to use pointers like this in C++ **at all**. But there's a need for `malloc()` in C++: interfacing with C. –  Aug 18 '17 at 13:38
  • @FelixPalmen That is correct. WinAPI comes to mind. – Ron Aug 18 '17 at 13:39
  • 1
    It is because you *print* the result of each element's assignment *before* you overwrite it in a following iteration. – Weather Vane Aug 18 '17 at 13:39
  • 1
    This is C, not C++. – Andrejs Cainikovs Aug 18 '17 at 13:40
  • 1
    @AndrejsCainikovs if OP uses C++, it doesn't matter whether it's a "good idea" to write such code in C++. It's still C++. –  Aug 18 '17 at 13:41
  • 2
    @AndrejsCainikovs The OP said they were using C++ and `int **p = (int **)malloc(2 * sizeof(int *));` is the C++ way to use `malloc` so I would stick with C++ unless the OP says otherwise. – NathanOliver Aug 18 '17 at 13:43
  • the example i'm using coming from the cppinstitute.org CLA course chapter 5 test – anonymous007 Aug 18 '17 at 13:51
  • 2
    @anonymous007 if they are presenting that in a C++ course you should consider to not continue that course. _CLA: Programming Essentials in C_ So it looks like it's a C course. –  Aug 18 '17 at 13:58
  • they are presenting a course in C language – anonymous007 Aug 18 '17 at 14:01
  • 1
    Then why do you use the C++ tag instead of the C tag? –  Aug 18 '17 at 14:02
  • 2
    I don't know differences between c and c++ however from the discussions above can tell there is a lot of difference :D – anonymous007 Aug 18 '17 at 14:05
  • 1
    @anonymous007 yes there is. If this is supposed to be C, by all means use a C compiler (and do the tag edits yourself). Then you can remove the redundant (in C, not in C++) casts on the `malloc()` calls. –  Aug 18 '17 at 14:07
  • Is visual studio 2017 community is good to use to compile c code? or is it supposed to be used with c++? – anonymous007 Aug 18 '17 at 14:16
  • @anonymous007 there's a C compiler coming with it, but it only fully conforms to C89 with **most** of C99, so very outdated. For windows, you might want to use MinGW, easily installable e.g. using [MSYS2](http://www.msys2.org/). AFAIK, it can be used with visual studio as well. –  Aug 18 '17 at 14:30

3 Answers3

5

First -- You are passing int* to printf(), which expects int (%d) , use (%p) for printing address instead.


Second --

p[1] = p[0];

The p[0] and p[1] pointers are both pointing to same address. Which means if you modify one row (p[0]), second (p[1]) will be modified as well.

And this is root of your problem

I am expecting p[0][0] = 0 as printed from the for loop, however it is equal to 1 and the value seems to be coming from p1[0], what am i missing here?

+---+---+
| 0 | 0 |
+---+---+
 ^ ^
 | |
 | -------------------- p[0]
 ---------------------- p[1]

Now you modify p[1], and it looks like this. In other words, values 0 and 1 of p[0] are overwritten

+---+---+
| 1 | 2 |
+---+---+
 ^ ^
 | |
 | -------------------- p[0]
 ---------------------- p[1]

Instead of p[1] = p[0], assign to p[1] (different) memory on heap as well.

p[0] = malloc(2 * sizeof(int)); 
p[1] = malloc(2 * sizeof(int));

And now p[0][0] is correctly 0.


Third -- recasting malloc is redundant and may lead to problems. Do i cast malloc return value?


In C++ you can(should) use built in operators new and delete.

Also dont forget to free() your allocated memory on heap.

kocica
  • 6,412
  • 2
  • 14
  • 35
0

Breaking it down line by line:

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

After this line executes, you have the following:

   +---+                +---+
p: |   | --------> [0]: |   | ---?
   +---+                +---+
                   [1]: |   | ---?
                        +---+

? indicates that p[0] and p[1] contain indeterminate pointer values (malloc does not initialize the memory it allocates); these pointer value are most likely not valid (i.e., do not correspond to the address of an object in your program).

p[0] = (int *)malloc(2 * sizeof(int));

After this line executes, you have the following:

   +---+                +---+              +---+
p: |   | --------> [0]: |   | ------> [0]: | ? |
   +---+                +---+              +---+
                   [1]: |   | ---?    [1]: | ? |
                        +---+              +---+

Again, ? represents an indeterminate value.

p[1] = p[0]; 

After this line executes, you have the following:

   +---+                +---+              +---+
p: |   | --------> [0]: |   | ---+--> [0]: | ? |
   +---+                +---+    |         +---+
                   [1]: |   | ---+    [1]: | ? |
                        +---+              +---+

Now, p[0] and p[1] are pointing to the same block of memory. Thus, p[0][0] and p[1][0] resolve to the same object. If we unroll your loop, we see the following sequence of assignments:

p[0][0] = 0 + 0;

   +---+                +---+              +---+
p: |   | --------> [0]: |   | ---+--> [0]: | 0 |
   +---+                +---+    |         +---+
                   [1]: |   | ---+    [1]: | ? |
                        +---+              +---+

p[0][1] = 0 + 1;

   +---+                +---+              +---+
p: |   | --------> [0]: |   | ---+--> [0]: | 0 |
   +---+                +---+    |         +---+
                   [1]: |   | ---+    [1]: | 1 |
                        +---+              +---+

p[1][0] = 1 + 0;

   +---+                +---+              +---+
p: |   | --------> [0]: |   | ---+--> [0]: | 1 |
   +---+                +---+    |         +---+
                   [1]: |   | ---+    [1]: | 1 |
                        +---+              +---+

p[1][1] = 1 + 1;

   +---+                +---+              +---+
p: |   | --------> [0]: |   | ---+--> [0]: | 1 |
   +---+                +---+    |         +---+
                   [1]: |   | ---+    [1]: | 2 |
                        +---+              +---+

This is why you're seeing the ouput you're seeing. The fact that p[0] and p[1] show the same value in your output should have been a big hint.

If p[1] is meant to point to a different array from p[0], then you'll need to allocate another block of memory for p[1] to point to:

p[1] = malloc( 2 * sizeof *p[1] );

As a matter of practice, you'll want to explicitly free everything you allocate, even if your program is about to exit - it's just a good habit to get into:

free( p[0] ); // free each p[i] *before* freeing p
free( p[1] ); // assuming you added the malloc for p[1]
free( p );

If this is meant to be C code, lose the cast on the malloc calls - it's unnecessary, it just adds visual clutter, and under C89 could mask a bug if you forgot to include stdlib.h. A much better way to write those calls is

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

and

p[0] = malloc( 2 * sizeof *p[0] );

If this is meant to be C++ code, use new and delete instead of malloc and free.

John Bode
  • 119,563
  • 19
  • 122
  • 198
-1

I'm answering my own question as it is getting a lot of irrelevant discussions.

as i have
pointed row 1 to share the same allocated memory with row 2 here -> p[1] = p[0]; the "for" loop overwrites the values in the first row with values from the second row.

The example function is coming from the cppinstitute.org CLA course chapter 5. Hope this will give a little more clarity to somebody like me in the future.

anonymous007
  • 319
  • 1
  • 3
  • 12
  • 1
    Its not _"irrelevant discussions"_. We are just giving you more advices what are you doing wrong. – kocica Aug 18 '17 at 13:57
  • You just completely do not understand pointers. Start from something easier just the basics - then progress to the more complicated problems etc etc. I do not think that anyone can explain it to you as you do not have the basic knowledge (you have missed several lessons before this one). – 0___________ Aug 18 '17 at 14:29
  • @PeterJ I do not agree with you. Even though i'm still learning the pointers. But never mind. – anonymous007 Aug 18 '17 at 14:32
  • @anonymous007 there is nothing to agree. It is obvious. less YT more books - my advice – 0___________ Aug 18 '17 at 14:33