user6542423
user6542423

Reputation:

Cannot assign values to struct members

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;
}

Upvotes: 1

Views: 905

Answers (2)

Cherubim
Cherubim

Reputation: 5457

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...

Upvotes: 2

kfsone
kfsone

Reputation: 24249

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

Upvotes: 1

Related Questions