Copy-Program works for small files, gives segmentation fault on lager files

Question

To learn some low level C magic, I tried to implement different methods to copy the content of one file to another.

The following method uses a buffer, where I save chunks of data, e.g. 512 bytes. Then I write FROM this buffer into my destination file. Allegedly this is more efficient than copying the whole file directly.

When I use a small buffer, e.g. 512 Bytes, this works fine. If I use a big buffer, e.g. 10000000 Bytes, roughly 10MB, I get a segmentation fault.

I am running on Ubuntu. I tried stuff with malloc, but nothing really seams to work for me. I guess the fix will be easy, once I know it.

According to my debbuger the error is in this line:

if (stat(to, &statBuffer) == -1)

I left in a lot of the error checking I am doing. Here is the code I run:

void copyUsingBuffer(char from[], char to[], long unsigned int bufferSize)
{
    struct stat statBuffer;
    int sourceFD, destFD;
    char buffer[bufferSize / sizeof(char)];
    int EndOfFileErreicht = 0;

    // Dateien pruefen
    if (stat(to, &statBuffer) == -1)
    {
        if (errno != ENOENT)
        {
            perror("Fehler beim Pruefen der Zieldatei");
            exit(EXIT_FAILURE);
        }
    }
    else
    {
        if (!(S_ISREG(statBuffer.st_mode)))
        {
            printf("Die Zieldatei ist keine regulaere Datei\n");
            exit(EXIT_FAILURE);
        }
    }

    // Dateien oeffnen
    sourceFD = open(from, O_RDONLY);
    if (sourceFD == -1)
    {
        perror("Fehler beim Oeffnen der Quelldatei");
        exit(EXIT_FAILURE);
    }
    if (fstat(sourceFD, &statBuffer) == -1)
    {
        perror("Fehler beim Pruefen der Quelldatei");
        exit(EXIT_FAILURE);
    }
    if (!(S_ISREG(statBuffer.st_mode)))
    {
        printf("Die Quelldatei ist keine regulaere Datei\n");
        exit(EXIT_FAILURE);
    }

    destFD = open(to, O_WRONLY | O_CREAT | O_TRUNC, statBuffer.st_mode);
    if (destFD == -1)
    {
        perror("Fehler beim Oeffnen der Zieldatei");
        exit(EXIT_FAILURE);
    }

    while (EndOfFileErreicht == 0)
    {
        int geleseneBytes = read(sourceFD, buffer, sizeof(buffer));
        if (geleseneBytes == -1)
        {
            perror("Fehler beim Lesen aus der Quelldatei");
            exit(EXIT_FAILURE);
        }
        if (geleseneBytes < bufferSize)
        {
            // EOF wurde erreicht, also Flag setzen
            EndOfFileErreicht = 1;
        }


        if (write(destFD, buffer, geleseneBytes) == -1)
        {
            perror("Fehler beim Schreiben in die Zieldatei");
            exit(EXIT_FAILURE);
        }
    }

    close(sourceFD);
    close(destFD);
}

I would expect that if I enter a 500MB text file, that my program copies this file to its destination.

I realise that there are different ways to implement file copying. I already implemented some of those. In this case I am restricted to using write, read, and the buffer. Also bufferSize NEEDS to be a variable. The naive way i have using malloc would be:

 char* buffer = malloc(bufferSize);

But when i use this the file will be corrupted.

Answer 1

Found the solution, after trying a few things. I replace

char buffer[bufferSize / sizeof(char)];

with

char* buffer = malloc(bufferSize);

Using malloc was already suggested. MY mistake was that i didnt replace

int geleseneBytes = read(sourceFD, buffer, sizeof(buffer));

Making those changes my program works for all file-sizes. Thanks for the help everybody!

int geleseneBytes = read(sourceFD, buffer, bufferSize);

Answer 2

You need to also change the line:

int geleseneBytes = read(sourceFD, buffer, sizeof(buffer));

To:
    int geleseneBytes = read(sourceFD, buffer, bufferSize);

Because the sizeof(buffer) is the size of a pointer, not the size of what it points to. Since you are malloc()ing it, you need to provide this.

ps:

char buffer[bufferSize/sizeof(char)];

is a bit cring inducing. Sizeof(char) is 1.

Answer 3

If someone has a better solution, e.g. something with malloc, it would also be a big help.

Option 1:

Don't make bufferSize an input to the function. Use a hard-coded size that you know will not cause buffer overflow, such as std::numeric_limits<unsigned short>::max().

Option 2:

Use std::vector insteady of an array.

// char buffer[bufferSize / sizeof(char)];
std::vector<char> buffer(bufferSize);

Then use buffer.data() when you need to access the data of the object.

Update (since C++ tag was removed)

Don't make bufferSize an input to the function. Use a hard-coded size that you know willnot cause buffer overflow, such as USHORT_MAX.

void copyUsingBuffer(char from[], char to[])
{
    int bufferSize = USHORT_MAX
    char buffer[USHORT_MAX];

    struct stat statBuffer;
    int sourceFD, destFD;

    ...
}