Segmentation fault when trying to create a buffer of 100MB

Question

I am trying to write a large binary file into a buffer in a C++ program. GDB always gets a segfault after trying to create a buffer the same size as the file read. It either fails on fclose(pf), rewind, or f(open) which leads me to believe that there is something wrong when I am trying to create the buffer. My code segment is as follows.

static int fileTransfer(struct mg_connection *conn, char * filename){

    FILE *fp = fopen(filename, "r"); 

    fseek(fp, 0, SEEK_END);
    int size = ftell(fp);

    char buf[size];

    fclose(fp); 

    // This is an attempt to stop a segment fault from rewind. 
    fp = fopen(filename, "r"); 

    conn->connection_param = (void *) fp;
    size_t n = 0;

    if(fp != NULL)
    {
        n = fread(buf, 1, sizeof(buf), fp);

        mg_send_data(conn, buf, n);

        if(n < sizeof(buf) || conn->wsbits != 0)
        {
            fclose(fp);
            conn->connection_param = NULL;
        }
    }

return 1;
}

I have tried putting print statements in this code but they don't print to the console as they are running in a separate thread. Can someone give me some insight on why this segfault is happening, or some suggestions on how to make this code more efficient.

I should note that this code works properly on 1 and 10 MB files but not on anything larger.

allocating 100megs on the stack? youll get document.location.hostname.substr(0,13). try to use malloc — Valerij
– Valerij, Commented Apr 27, 2014 at 18:06
Why was the c++ tag changed`? This is legal c++ and called such in the question! — danielschemmel
– danielschemmel, Commented Apr 27, 2014 at 18:08
Also note that fseek, fclose and fopen don't always succeed. Robust code should check for such errors. — Edward
– Edward, Commented Apr 27, 2014 at 18:09
Well, the dynamically sized arrays are an g++ extension, but still... — danielschemmel
– danielschemmel, Commented Apr 27, 2014 at 18:10
This code looks closer to C99, not C++. VLAs aren't standard C++, and they use struct type*, fseek(), et al. — Robert Mason
– Robert Mason, Commented Apr 27, 2014 at 18:10

Wagner Patriota · Accepted Answer · 2014-04-27 18:08:38Z

3

never do this:

int size = ftell(fp);

char buf[size];

You are creating size on the STACK, not on the heap.... 100MB on the stack will not work.

AND... size must be a constant number, not a number coming from ftell(). I even don't know how it is compiling...

What you have to to is to allocate memory using malloc() or new operator.

static int fileTransfer(struct mg_connection *conn, char * filename){

    FILE *fp = fopen(filename, "r"); 

    fseek(fp, 0, SEEK_END);
    int size = ftell(fp);

    char * buf = new char[size];  // fix also here!

    fclose(fp); 

    // This is an attempt to stop a segment fault from rewind. 
    fp = fopen(filename, "r"); 

    conn->connection_param = (void *) fp;
    size_t n = 0;

    if(fp != NULL)
    {
        n = fread(buf, 1, size, fp); // fix also here!

        mg_send_data(conn, buf, n);

        if(n < size || conn->wsbits != 0)
        {
            fclose(fp);
            conn->connection_param = NULL;
        }
    }

    delete [] buf; // and you have to deallocate your buffer

    return 1;
}

answered Apr 27, 2014 at 18:08

Wagner Patriota

5,74428 silver badges49 bronze badges

Sign up to request clarification or add additional context in comments.

9 Comments

Robert Mason Over a year ago

You never delete buf - though that's OK in this instance as the OS will reclaim the memory this is a bad habit for people to get into unless they know what they are doing... probably bad practice on a site like SO.

Robert Mason Over a year ago

Nevermind, you edited that a second before I posted the comment.

JME Over a year ago

@WagnerPatriota Your solution worked, quick note on the ftell, it works because fseek reads to the end of the file to get the file size. If there is a better way to get the size of an opened file I am all for suggestions.

PaulMcKenzie Over a year ago

@JME Why couldn't you just have used std::vector<size>? It is legal C++, as opposed to your usage of VLA.

Wagner Patriota Over a year ago

the way to the get file size is correct. what is not correct is to put this value inside the char buf[size] because this one is created on the stack. in other words, this value is created in compile time. it should not even compile. if your size is a dynamic value [and you got it correctly] you MUST allocate memory dynamically. not on the stack in this case.

|

Robert Mason · Accepted Answer · 2014-04-27 18:08:08Z

What's going on is actually the namesake of this site! Basically, what is happening is your program is created it has a set amount of memory allocated for the stack.

When you create char buf[size], you are using a C99 feature called a variable length array (VLA). This allocates space on the stack for buf. However, buf is too large for the stack, so your program fails.

In order to fix this problem, you should use char * buf; and then do buf = malloc(size). This will place buf on the heap, which is larger than the stack. It also lets you check if you do not have enough memory, by checking if malloc() returns NULL. You need to be sure to free(buf) before you exit though!

As a side note, you can check how much space you have on the stack by using the ulimit -s command.

danielschemmel · Accepted Answer · 2014-04-27 18:07:10Z

2

You are creating a buffer with automatic storage duration, which means it will be put on the stack by g++. The default stack size for any OS known to me is below 100 MB, meaning it will cause a segfault on system supporting them.

Try allocating your buffer with dynamic storage duration, which will place it on the heap.

answered Apr 27, 2014 at 18:07

danielschemmel

11.2k1 gold badge39 silver badges60 bronze badges

Comments

Chris Bogart · Accepted Answer · 2014-04-27 18:08:44Z

2

That seems like a lot to allocate on the stack. What if you put it on the heap instead?

char *buf = new char[size];

answered Apr 27, 2014 at 18:08

Chris Bogart

4722 silver badges8 bronze badges

Comments

PaulMcKenzie · Accepted Answer · 2014-04-27 18:29:10Z

Use std::vector. Then you don't have the issues of stack space, or the other issue of writing non-standard C++ code:

#include <vector>
//...
static int fileTransfer(struct mg_connection *conn, char * filename)
{
    FILE *fp = fopen(filename, "r"); 
    fseek(fp, 0, SEEK_END);

    int size = ftell(fp);
    std::vector<char> buf(size);
    fclose(fp); 

    fp = fopen(filename, "r"); 

    conn->connection_param = (void *) fp;
    size_t n = 0;

    if(fp != NULL)
    {
        n = fread(&buf[0], 1, buf.size(), fp);
        mg_send_data(conn, &buf[0], n);
        if(n < buf.size() || conn->wsbits != 0)
        {
            fclose(fp);
            conn->connection_param = NULL;
        }
    }
    return 1;
}

Collectives™ on Stack Overflow

Segmentation fault when trying to create a buffer of 100MB

5 Answers 5

9 Comments

Comments

Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

9 Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related