0
#include<stdio.h>
#include<stdlib.h>
void add(char **p);
void print(char **p);
int cnt=0;
main()
{
  int option;
  char **p=NULL;
while(1)
{
  printf("------MENU-----\n");
  printf("1>input\n 2>print\n3>exit\n");
  printf("enter ur choice\n");
  scanf("%d",&option);getchar();
 switch(option)
 {
    case 1: add(p);
            break;
    case 2: print(p);
            break;
   case 3: return;
   default: printf("Invalid option\n");

 }
}
}
void add(char **p)
{
  int i;
  p=(char**)realloc(p,(cnt+1)*sizeof(char*));
  if(p==NULL)
  {
        printf("Error: memory not available\n");
        return;

  }
  p[cnt]=NULL;
  p[cnt]=(char*)realloc(p[cnt],20*sizeof(char));

  puts("enter a name");
  gets(p[cnt]);
  cnt++;
  printf("cnt=%d\n",cnt);
}
void print(char **p)
{
  int i;
  for(i=0;i<cnt;i++)
  printf("p[%d]=%s\n",i,p[i]);
}

In the above code, I am making a database of names. For this I am using dynamic memory allocation. I am allocation memory for 2D-array using array of pointers method. When I am executing this program on gcc compiler, I am getting segmentation fault. I am not understanding why is it happening?Could you please tell me where the bug is?

Jhansi Rani
  • 449
  • 1
  • 5
  • 11
  • On top of what's already been mentioned in the answers below about variable `p` being local in function `add`, I'm pretty sure that you must initialize `p = malloc(...)` to begin with, since function `realloc` expects (as input argument) a pointer to a memory segment previously allocated with either `malloc` or `realloc`. – barak manos Dec 03 '14 at 12:34
  • Some hints: use a debugger, consistently indent your code, avoid `gets`, do not cast the return values of `malloc`/`realloc`, `sizeof(char)` is always 1. – undur_gongor Dec 03 '14 at 12:34
  • @barakmanos: My C standard says "If `ptr` is a null pointer, the `realloc` function behaves like the `malloc` function." But still, if `print` is called without prior `add`, the pointer is 0. – undur_gongor Dec 03 '14 at 12:38
  • 1
    @barakmanos - `realloc` can do literally __everything__ which `malloc` and `free` can do (including initial allocations). Which is an argument for never using `realloc` rather than for never using `malloc` or `free`! Whoever designed `realloc` had forgotten about 'do one thing and do it well' as a design principle. – AAT Dec 03 '14 at 13:08
  • @AAT `realloc()` may "copy" the contents of memory to another new logical location without changing the physical address for it knows the source will be free'd. Hence no `memcpy()` need to occur. How would `malloc()` and `free()` perform this efficient re-allocation? – chux - Reinstate Monica Dec 03 '14 at 15:52
  • `gets(p[cnt]);` is prone to input overflow. Suggest using `fget(p[cnt], 20, stdin);`. Note that a `'\n'` may be included near the end of the string. – chux - Reinstate Monica Dec 03 '14 at 15:56
  • @chux - if you happen to be in a situation where that level is optimisation is significant, then the risks associated with `realloc` may be justified, __if__ you can show that it does perform better than `malloc - memcpy - free` for your case. But using it as a reason for __starting__ with `realloc` sounds like premature optimisation to me. – AAT Dec 04 '14 at 08:58

5 Answers5

1

p in main is handed over to add by value. add modifies a local copy then, but not the original p.

Besides the terrible formatting and everything you need to hand a pointer to your main's p to add:

...

  case 1: add(&p);
...

void add(char ***p)
{
  int i;
  *p = realloc(*p,(cnt+1)*sizeof(char*));
  if(*p==NULL)
  {
        printf("Error: memory not available\n");
        return;

  }
  (*p)[cnt]=NULL;
  (*p)[cnt]=realloc((*p)[cnt],20*sizeof(char));

  puts("enter a name");
  gets((*p)[cnt]);
  cnt++;
  printf("cnt=%d\n",cnt);
}
undur_gongor
  • 15,657
  • 5
  • 63
  • 75
1

In main, all you do is to assign p = NULL and then use this NULL pointer in print, which causes the segfault. Note that add(p) is equivalent to add(NULL) and does not change p. Did you mean to pass an address with add(&p)? If so, you need to fiddle a bit with the number of *.

Jens
  • 69,818
  • 15
  • 125
  • 179
0

Pass by reference. You need to pass the address of your pointer to make sure the changes in function add() is reflected in main(). Avoid using gets()

In this case

add(&p);

Accordingly your add() function definition should change to handle this. Or the other way is for the add function to make the required allocations and return that address to your pointer

char **add();

Check the code below:

char  **add(char **p)
{
  int i;
  p=(char**)realloc(p,(cnt+1)*sizeof(char*));
  if(p==NULL)
  {
        printf("Error: memory not available\n");
        return;

  }
  p[cnt]=NULL;
  p[cnt]=(char*)realloc(p[cnt],20*sizeof(char));

  scanf("%s",p[cnt]);
  cnt++;
  printf("cnt=%d\n",cnt);
  return p;
}

So your call should be:

p = add(p);
Gopi
  • 19,784
  • 4
  • 24
  • 36
0

A copy of 2D pointer p is created in the add function, and updating this pointer will not be reflected in the main function. A quick solution to this problem is to return the 2D pointer 'p' from the add function.

char** print(char **p)
{
    ....
    ....
    return p;
}
Lavish Kothari
  • 2,211
  • 21
  • 29
0

Apart from the other things, I don't think anyone's mentioned the realloc trap of reallocation failure. Suppose this call

p=(char**)realloc(p,(cnt+1)*sizeof(char*));

fails to allocate the new memory: what happens then? Yes, you will get NULL returned: but the memory which was allocated, and which pis pointing to, does not get free'd. Instant memory leak.

You must call realloc like this:

char* pTemp = realloc(p, cnt + 1);
if(pTemp != NULL)
{
    p = pTemp;             // Now safe to update p
}
else
{
    // Error handling as required, possibly including freeing p
}

Yet another reason not to use realloc. It doesn't really by you very much over doing the buffer copies yourself IMHO.

AAT
  • 3,286
  • 1
  • 22
  • 26