Cannot assign values to struct members

Question

I have dynamically allocated a structure array representing a series of points. However, when I try to assign values to its members, the program assigns different values than it is supposed to. What am I doing wrong?

Output of the program:

xy[0].x= 0 
xy[0].y= 0 
xy[1].x= 1 
xy[1].y= 1 
xy[2].x= 2 
xy[2].y= 2 
xy[3].x= 1041 
xy[3].y= 0

Code:

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

int n;

struct point{
  int x,y;
};
typedef struct point Point; 


void resize(Point*xy) 
{
    xy=(Point*)realloc(xy,sizeof(Point));

    for (n=0;n<4;n++)
    {
    xy[n].x=n;
    xy[n].y=n;
    }   
}


int main () 
{
   Point *xy;
   xy=(Point*)malloc(2*sizeof(Point));

   for (n=0;n<2;n++){
       xy[n].x=n;
       xy[n].y=n;
   }       

   resize(xy) ; 
   for (n = 0; n<4; n++) {
       printf("xy[%i].x= %i \n",n,xy[n].x);
       printf("xy[%i].y= %i \n",n,xy[n].y);
   }       

   free(xy);
   return 0;
}

In `resize()`, you only `realloc()` enough room for one struct... and save the result to `xy` from the argument list, so if the address changes the `xy` in `main()` will still contain the old address (though since you're shrinking the array, `realloc()` will likely return the same address anyway). — Dmitri, Jul 02 '16 at 22:15
You should either pass a `Point **` instead of `Point *` to `resize()`, or return `Point *` to assign back to `xy` in `main()`. — Dmitri, Jul 02 '16 at 22:19
Always check the result of functions which can yield a critical error like `realloc`. And don't cast the result of `malloc` & friends in C! — too honest for this site, Jul 03 '16 at 00:06

score 2 · Answer 1 · edited May 23 '17 at 12:22

2

What am I doing wrong?

The problem is that while you are reallocating the size of xy int resize() function, you are not allocating sufficient memory:
```
void resize(Point*xy) {
xy = (Point*) realloc( xy, sizeof(Point));
```
here, realloc(xy,sizeof(Point)); allocates memory for only 1 structure thus you can access only xy[0].x and xy[0].y
but, in the for loop you access xy[1], xy[2] and xy[3] ... this is

accessing out of bounds and this results in undefined behavior

and thus, you get undefined output.

Solution :

allocate sufficient memory using realloc()

As you've iterated the for loop 4 ( n= 0 to 3 ) times, I suppose you need to allocate memory for 4 structures
Just multiply 4 to the second argument of realloc() in the resize() function
```
xy = (Point*)realloc( xy, 4*sizeof(Point)); //multiply with 4
```
here sufficient amount of memory is allocated to store 4 structures

output : (after the change)

xy[0].x= 0 
xy[0].y= 0 
xy[1].x= 1 
xy[1].y= 1 
xy[2].x= 2 
xy[2].y= 2 
xy[3].x= 3 
xy[3].y= 3

The result you get is as expected :)

Suggestions :

Firstly,

Don't cast the value of malloc() (and realloc()) as they return void type which gets automatically converted to the data-type of pointer that it's getting assigned to.

so instead of :

xy = (Point*)realloc(xy,4*sizeof(Point)); //same for malloc dont cast

use
```
xy= realloc(xy,4*sizeof(Point));
```

For further reading see this : click

and then,

Don't use int main(), instead use int main(void) as no arguments are being sent into main() function.But, in first case that is int main(), any number of arguments can be sent into main() function...

edited May 23 '17 at 12:22

Community

1
1

answered Jul 02 '16 at 23:34

Cherubim

5,287
3
20
37

1

The indexing brackets dereference it for him -- `xy[0].x` will work, `xy[0]->x` will not. – Dmitri Jul 02 '16 at 23:43
@Dmitri thanks figured that out... thanks for the mention :) i've edited the answer – Cherubim Jul 02 '16 at 23:58
@PintoDoido Pleasure helping you :) ! If this has helped answering your question, you can mark it as answer for future viewers to know that this explains and solves the problem.. – Cherubim Jul 03 '16 at 12:28
main can receive 0, 2 or 3 arguments. The 3 arguments variant is int main(int argc, char **argv, char **envp). (just to be pedantic). I'll only ask whether this is gcc-specific or is portable. – Paul Stelian Jul 03 '16 at 12:30
1

@PaulStelian true.. But as OP is not sending any arguments into main() i thought it'd be better to suggest him on using int main(void) – Cherubim Jul 03 '16 at 12:33
@CherubimAnand I have never done this and anyway I'm more accustomed with C++ where the two declarations are equivalent. Also, C99 is compatible with K&R or has the same semantics as C++? You imply the former... – Paul Stelian Jul 03 '16 at 12:36
1

@PaulStelian In C++, there is no difference, both are same. Both definitions work in C also, but the second definition with void is considered technically better as it clearly specifies that main can only be called without any parameter. For more see this: http://www.geeksforgeeks.org/difference-int-main-int-mainvoid/ – Cherubim Jul 03 '16 at 12:42
@CherubimAnand Thanks for clarifying. – Paul Stelian Jul 03 '16 at 12:46
1

@PaulStelian No problem :) and by the way no need to return 0 at the end of main()..as most of the modern compilers automatically return zero on reaching end of main. Apparently this info was shared to me by other user, so i just pass the knowledge around :) – Cherubim Jul 03 '16 at 12:47
@CherubimAnand Of course this is only implied with the main() function in C++ (in C I've seen implicit values vary and be meaningless), as a special case. – Paul Stelian Jul 03 '16 at 13:32

score 1 · Answer 2 · answered Jul 03 '16 at 00:01

There are two problems with your resize function:

void resize(Point*xy) {
    xy=(Point*)realloc(xy,sizeof(Point));  // changes local variable

    for (n=0;n<4;n++)
    {
        xy[n].x=n;
        xy[n].y=n;
    }   
}

Firstly, you shrink the size of the allocation from 2*sizeof(Point) to sizeof(Point). reallocs second argument is the new total size of the allocation, not how much you want to add.

Secondly, your function only modifies the local variable xy, it does not return it or make the caller aware. In main:

   // imagine xy = 0x10000.
   resize(xy) ;  // frees 0x10000 and allocates new memory
   // xy here is still 0x10000

You need to either take a Point** pointer in resize or you need resize to return it's local xy value:

Point* resize(Point*xy) {
    int n;
    xy=(Point*)realloc(xy, 4*sizeof(Point));

    for (n=0;n<4;n++)
    {
        xy[n].x=n;
        xy[n].y=n;
    }   

    return xy;
}

or

void resize(Point** xyp) {
    int n;
    *xyp=(Point*)realloc(*xyp, 4*sizeof(Point));

    for (n=0;n<4;n++)
    {
        (*xyp)[n].x=n;
        (*xyp)[n].y=n;
    }   
}

Example:

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

typedef struct {
    int x, y;
} Point;

void resize(Point** xyp) {
    int n;
    *xyp=(Point*)realloc(*xyp, 4*sizeof(Point));

    for (n=0;n<4;n++)
    {
        (*xyp)[n].x=n;
        (*xyp)[n].y=n;
    }   
}

int main () {
    int n;
    Point *xy;
    xy=(Point*)malloc(2*sizeof(Point));

    for (n=0;n<2;n++)
    {
        xy[n].x=n;
        xy[n].y=n;
    }       

   resize(&xy) ; 

   for (n = 0; n<4; n++) {
        printf("xy[%i].x= %i \n",n,xy[n].x);
        printf("xy[%i].y= %i \n",n,xy[n].y);
   }       

   free(xy);

   return 0;
}

http://ideone.com/gIntgl

Cannot assign values to struct members

2 Answers2