C - Segmentation Fault when closing a file

Question

I've come across a weird issue where I receive a segmentation fault when trying to close the file. Data is being written to the file correctly, could there be some sort of race condition happening between fflush and fclose?

//main.c

#define filename "/home/test.txt";
char fbuf[255];
sprintf(fbuf, "%s, %f, %f,%f \n", "big", 8.0,0.0,0.8);

sendFile(&fbuf, sizeof(fbuf), (void *)filename);

static void
sendFile( void *data, int size, char *pName)
{
    FILE *pFile = fopen(pName,"a");
    char *buf = NULL;
    buf = (char *)malloc(255);
    memcpy(buf, data, sizeof(char *)*size);

    if(pFile == NULL) {
        logger(LOG_INFO, "Error opening file\n");
}
    else {
        fwrite(buf, 1, strlen(buf), pFile);
        fflush(pFile);
        fclose(pFile);
    }
    free (buf);
}

Any help or suggestions are greatly appreciated.

Answer 1

I think the problem is in memcpy(buf, data, sizeof(char *)*size).

Shouldn't it be simply memcpy(buf, data, size)?

Looking at your example, fbuf (i.e. data) is 255 chars, and buf is 255 chars as well. But the memcpy is copying more than one thousand chars effectively writing garbage into the heap, with unpredictable results.

Answer 2

Here is a version of the code that works, with embedded comments about why things are different from the posted code.

Note: On linux (and other OSs) the /home directory is not writeable without administrative privileges. so the call to fopen() will always fail.

//main.c

#include <stdio.h>  // fwrite, fopen, fclose, perror
#include <stdlib.h> // exit, EXIT_FAILURE
#include <string.h> // strlen, sprintf, memcpy

// note: no comments at end of #define statement as the comment would be copied into the code
// note no ';' at end of #define statement
#define filename "/home/test.txt"
 // to avoid using 'magic' numbers in code
 // and this name used throughout the code
#define BUF_SIZE (255)

// prototypes
void sendFile( char *, char *); // so compiler does not make incorrect assumptions
                                // and no second parameter needed

char fbuf[BUF_SIZE] = {'\0'}; // avoid 'magic' numbers
                              // avoid placing trash into output file
                              // however, will place many NUL bytes

// main() function to make executable platform for testing
int main( void )
{
    sprintf(fbuf, "%s, %f, %f,%f \n", "big", 8.0, 0.0, 0.8);
     // changed 3rd parameter type to match actual function parameter type
    sendFile(fbuf, filename); // no '&' because in C
                              // array name degrades to address of array
    return 0;
}

void sendFile( char *data, char *pName) // use actual parameter types
{
    FILE *pFile = fopen(pName,"a");
    if(pFile == NULL)  // always check for error immediately after call to system function, not later
    { // then fopen failed
         perror( "fopen failed" );
        //logger(LOG_INFO, "Error opening file\n");
        exit( EXIT_FAILURE); // exit, so no other code executed
    }

    // implied else, fopen successful

    char *buf = NULL;
    if(NULL == (buf = malloc(BUF_SIZE) ) ) // don't cast returned value
    { // then malloc failed   -- always check for error immediately after call to system function
        perror( "malloc for output buffer failed");
        //logger(LOG_INFO, "malloc for BUF_SIZE failed");
        fclose( pFile); // cleanup
        exit(EXIT_FAILURE);  // exit, so no other code executed
    }

    // implied else, malloc successful

    memcpy(buf, data, BUF_SIZE); // using original 3rd parameter would move 4 times as many bytes,
                             // I.E. past the end of the source buffer and past the end of the destination buffer
                             // resulting in undefined behaviour, leading to a seg fault event
                             // this memcpy() and the destination buffer
                             // are unneeded as first passed in parameter
                             // contains ptr to the source buffer
                             // which can be used in the call to fwrite()

    fwrite(buf, BUF_SIZE, 1, pFile); // buf will have NUL byte immediately after 'big' so strlen() would return 3
                                     // and the syntax for fwrite is source buffer, size of one item, number of items, FILE*
                                     // and this parameter order is best
                                     // because, if error checking,
                                     // can just compare returned value to 1
    fflush(pFile);  // not actually needed as the fclose() performs a flush
    fclose(pFile);

    free (buf);     // original code did this even if malloc had failed
                    // and even if the fopen had failed
                    // which would have corrupted the 'heap' leading to a seg fault event
}