-1

i have decleard a structure and allocate some memory too . using a function i update datas . i got error segmentation fault when i acssing data.

This is my code

In headerfile :

typedef struct
{
int member;
char *name;
}place;

void update(place **,int);
void display(place **,int);

in function

static memallocate(place **ptr,int viname,int index)
{

 ptr[index]=(place *)malloc(sizeof(place));
 ptr[index]->name=(char *)malloc(viname*sizeof(char *));

}

void update(place **ptr,int index)
{
 ---read string value "na" find the strlen as "pp"---
 memallocate(ptr,pp,index);
 ptr[index]->name=na;

}

void display(place **ptr,int index)
{
 int i;
 for(i=0;i<index;i++)
  {
    printf("%s\n",ptr[i]->name);
    printf("%s\n",ptr[i]->country);
  }
}

in main file :

void main()
{
 int index=0;
 place *pla[5]={NULL};
 while(index<2)
 {
    update(&pla[index],index);
    index++;
 }
 display(pla,index);
}

my problem is i got segmentation fault when acessing function display and can't print datas ptr[0]->name,ptr[0]->country,ptr[1]->name,ptr[1]->country ..why this happen ? any memory fault . I got printing when i use printf after each updation .

  • 1
    Would help greatly if you posted something that compiles. Preferably the actual code you are getting a segfault from. – M.M Mar 15 '15 at 11:48

2 Answers2

2

I see two mayor issues here.

1st

Here

static void memallocate(place **ptr,int viname,int index)
{
  ptr[index]=(place *)malloc(sizeof(place));
  ptr[index]->name=(char *)malloc(viname*sizeof(char *));  
}

you allocate too much memory. It shall be

static void memallocate(place ** ptr, int viname, int index)
{
  ptr[index] = malloc(sizeof(place));
  ptr[index]->name = malloc(viname * sizeof(char));  
}

or even better:

static int memallocate(place ** ptr, size_t viname, size_t index)
{
  int result = 0;

  if (NULL == ptr)
  {
    result = -1;
    errno = EINVAL;
  }
  else
  {
    ptr[index] = malloc(sizeof *ptr[index]);
    if (NULL == ptr[index])
    {
      result = -1;
    }
    else
    {
      ptr[index]->name = malloc(viname * sizeof *(ptr[index]->name));  
      if (NULL == ptr[index]->name)
      {
        result = -1;
        free(ptr[index]);
      }
    }
  }

  return result;
}

2nd

Then here (assuming na to be a char* properly initilaised to reference a C-"string")

void update(place **ptr,int index)
{
  ---read string value "na" find the strlen as "pp"---
  memallocate(ptr,pp,index);
  ptr[index]->name=na;
}

you overwrite what you just assigned to name. To copy a C-"string" use strcpy().

int update(place ** ptr, size_t index)
{
  ---read string value "na" find the strlen as "pp"---
  int result = memallocate(ptr, pp, index)
  if (-1 == result)
  {
    perror("memallocate() failed");
  }
  else
  { 
    strcpy(ptr[index]->name, na);
  }

  return result;
}

Then call it like this:

int main(void)
{
  size_t index = 0;
  place * pla[5] = {NULL};

  /* Loop over all array's elements. */
  while (index < sizeof pla/sizeof *pla)
  {
    update(pla, index);
    ++index;
  }

  ...
}

Notes:

  • Always check the outcome of relevant function calls (here malloc()) and design your functions to be able to pass failures up to the caller.
  • Do not cast the result of malloc(), calloc() and realloc() in C. It is not needed nor recommended.
  • Prefer using size_t over int for memory sizes and indexes. size_t does not waste a bit for negative numbers and it is guaranteed to be large enough to address any arrays' element or represent any memory size. sizeof as well as strlen() return size_t not int for example.
Community
  • 1
  • 1
alk
  • 69,737
  • 10
  • 105
  • 255
0

When you call your update(), you are passing a place ** of the current index as argument.

However, you nevertheless pass index too and later in your memallocate() allocate memory as if it was a pointer to the place *[].

So it should help to remove the parameter index from update() and memallocate() and change the memory allocation to something like:

*ptr = (place *)malloc(sizeof(place));
*ptr->name = (char *)malloc(viname*sizeof(char *));
nlu
  • 1,903
  • 14
  • 18
  • At least, I don't see how calling `update(&pla[index],index);` in `main()` and later allocating the memory to some offset of it can be correct. If it was to be done with passing the index, then the first argument should point to the array: `update(pla, index);` – nlu Mar 15 '15 at 12:19
  • You are correct, either pass the element directly or pass the array's root and the element's index. I fully missed the code for the OP's `main()` (had it been added later?). I assumed the code was called as per my updated answer. Please excuse, I take back my comment. – alk Mar 15 '15 at 12:35