malloc and realloc segmentation fault

Question

I'm working on linux and using as compiller gcc. I'm make some expriences with the functions malloc and realloc try to understand how it works. But when I execute the program give me segmentation fault. next my code:

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

 int main(){
  register int cont=1;
  int i,n,*a;
  a=(int*)malloc(sizeof(int));
  scanf("%d",&n);
  while(n!=0){
   if(a!=NULL)
    a=(int*)realloc(a,cont*sizeof(int));
   else 
    goto exit;
   a[i]=n;
   scanf("%d",&n);
   cont++;
   i++;
 }

 for(i=0;i<cont;i++)
  printf("%d\n",a[i]);
 free(a);
 return 0;

 exit: printf("No memory\n");
 free(a);
 return -1;



}

Why this don't work and whats wrong with my code?

Use this GCC command-line: `gcc -Wall -Werror -g main.c` This will enable warnings, and include debugging information in your executable. Then use `gdb a.out` to run your code in the debugger. Type `c` to "continue". It will then point out exactly where your program crashed. — Jonathon Reinhart, Jan 20 '15 at 07:05
[Please don't cast the result of `malloc()`](http://stackoverflow.com/a/605858/3233393). — Quentin, Jan 20 '15 at 07:12
Note that when you use `realloc()` as exemplified by `a = (int*)realloc(a, cont * sizeof(int));` you risk leaking memory if the reallocation fails. That's because when it fails, it overwrites `a` with NULL, which means you've lost the old pointer to the memory. Always use `int *new_a = (int *)realloc(a, cont * sizeof(int)); if (new_a != 0) a = new_a;` so that you can still release the previously allocated memory if something goes wrong. It doesn't matter much in this code; it does matter in general. It's generally best to report errors on the standard error stream, and to avoid using `goto`. — Jonathan Leffler, Jan 20 '15 at 07:26

score 7 · Accepted Answer · edited May 23 '17 at 12:20

7

i is never initialized, so a[i]=n; probably causes the segmentation fault. Change it to:

int i = 0;

There are some other improvement can be done to your code, e.g, don't cast the result of malloc, your use of goto doesn't look necessary in my opinion, and register keyword is probably useless.

edited May 23 '17 at 12:20

Community

1
1

answered Jan 20 '15 at 07:21

Yu Hao

119,891
44
235
294

thanks very much;but now the input: 2 3 4 5 6 7 8 0 result in output: 2 3 4 5 6 7 8 135137 any idea what this 135137 is? – warwcat Jan 20 '15 at 08:29

score 0 · Answer 2 · answered Jan 20 '15 at 10:41

In the while loop, after the user enters 0 , it is stored in n, you incremented the cont and when the while loop again checked the entry condition n != 0 it failed (because the value of n now is 0) and exited the loop without storing the n value to a, that is why you are getting the indeterminate value in the last place of your output.

When you are using realloc, you should not store the return value directly to the pointer variable which you are trying to increase the size, since realloc return NULL on failure, you end up erasing the handle/address to the memory buffer.

    register int cont = 1;
    int i = 0,n,*a;
    int *temp;
    a = (int*)malloc(sizeof(int));
    if(a == NULL) goto exit;
    while(1){
       scanf("%d", &n);
       temp = (int*)realloc(a, cont * sizeof(int));
       if(temp == NULL)
         goto exit;
       else
         a = temp;
       a[i++] = n;
       if(n == 0) // put the condition here if u want to store 0
         break;  
       else
         cont++;
     }

malloc and realloc segmentation fault

2 Answers2