Segmentation fault on program with scanf

3
votes

This is a small piece of code that I made while trying to understand how malloc and pointers work.

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

int *buffer (int count)
{
  int *buffer = malloc (count * sizeof(int));

  for (int i = 0; 0 <= i && i < count; i++)
    {
      buffer[i] = 0;
    }

  return &buffer;
}

int main ()
{
  int size = 0;
  int i = 0;
  scanf ("%d", &size);

  int *num = buffer (size);
  while (i < size)
    {
      scanf ("%d", &num[i]);
      i++;
    }
}

For some reason that I can't understand, I keep getting a segmentation fault. This error repeatedly happens on the last scanf() and I do not know why. I know i have to pass pointer to scan f and num is already a pointer so i thought that i would not need to include the &. But, I received a segmentation fault earlier if i do not. Also, I believe I have allocated the correct amount of space using malloc but I am not sure. Any help with what is happening here would be appreciated.

2
c
And learn how to use calloc and for loop. - llllllllll
This code will not compile cleanly on a C compiler. Either your current compiler is misconfigured or it is broken. You need to investigate why it doesn't give you a warning when you try to return the wrong type from the function. - Lundin
You use the &num[i], because the [i] dereferences the pointer buffer at the specific location. Then you need to get that address. ie You want the point to the value that num[i] points at. - matt

2 Answers

3
votes

You returned the pointer to the local variable buffer, which will banish on exiting the function buffer.

You should remove the & used in the return statement and return the pointer to allocated buffer.

Also checking whether malloc() is successful should be added.

2
votes

There are a couple of issues that I can see, and one of them is definitely a problem. In function, int *buffer (int count)

return &buffer;

This will return address of buffer which is already a local int * variable. So when the return happens, variable buffer would no longer be valid. Hence, the address is invalid.

One of the ways to go ahead as of now would be avoiding a function call buffer and using calloc(). Because, subject to availability, calloc() will allocate the memory of requested length, which will be initialized to 0 by default. Or, the other way would be making the buffer pointer a global variable.

Also, with existing implementation, there needs a piece of code which checks if malloc returned anything or not. That would indicate if the memory was allocated or not. Something like this would do:

int *buffer = malloc (count * sizeof(int));
if(buffer == NULL)
{
    // Some error handling
    return 0;
}

Additionally, I see the for loop which looks a bit weird than what it should look like:

for (int i = 0; 0 <= i && i < count; i++)

I take that you are trying to loop the count times and fill a 0 in buffer. This could have been achieved by

for (int i = 0; i < count; i++)

So, a malloc() is followed by en error-check and then followed by a for to fill the allocated memory with zeroes. So, using calloc makes life a lot easier.

Importantly, you allocate memory but you don't seem to have a code that de-allocates (frees) it. There are ample of examples to refer for doing that. I would recommend you to read concepts like Memory Leakage, Dangling Pointers and using valgrind or similar thing to validate the memory usage.

As a side-note and not a rule of thumb, always make sure that the names you use for variables are different than the names you use with functions. That creates a hell a lot of confusion. Going ahead with existing naming habit, you'll have a tough day when the code is reviewed.