Segmentation fault: 11 while trying to print array with 50k items

Question

I'm trying to print array with 50k items into a file but it could be done only if I set small numbers of items, e.g. 5k.

void fputsArray(int *arr, int size, char *filename)
{
    char *string = (char*)calloc( (8*size+1), sizeof(char) );
    for(int i = 0; i < size; i++)
        sprintf( &string[ strlen(string) ], "%d\n", arr[i] );

    FILE *output;
    char fullFilename[50] = "./";
    output = fopen(strcat(fullFilename, filename), "w");
    fputs(string, output);
    fclose(output);
    free(string);
}

size is 50000, defined in #DEFINE. This is working code. But if I delete 8 multiplying to size, that is I supposed to be working, doesn't work. I got in that situation Segmentation fault: 11 Why should I allocate 8 times more memory than I need?

Answer 1

Assuming the input to your function is all correct:

void fputsArray(int *arr, int size, char *filename)

Sizes should be given as size_t.

{
    char *string = (char*)calloc( (8*size+1), sizeof(char) );

The clearing of the memory (calloc) is unnecessary, malloc and setting string[0] = '\0' would suffice. sizeof( char ) is always 1 by definition. And you should not cast the result of an allocation.

Actually, the whole construct is unnecessary, but that's for later.

    for(int i = 0; i < size; i++)
        sprintf( &string[ strlen(string) ], "%d\n", arr[i] );

Not actually that bad, aside from string + strlen( string ) being simpler and that there should always be { } surrounding the statement. Still unnecessarily complex.

    FILE *output;
    char fullFilename[50] = "./";
    output = fopen(strcat(fullFilename, filename), "w");

A filename is always relative to the current working directory, so the "./" is unnecessary. You should however have checked the filename length before strcating it into a static buffer like that.

    fputs(string, output);

Ah, but you have not checked if the fopen actually succeeded!

    fclose(output);
    free(string);
}

All in all, I've seen worse. Whether your numbers actually fit your buffer is guesswork, though, and most importantly the whole memory shenanigans are unnecessary.

Consider:

void printArray( int const * arr, size_t size, char const * filename )
{
    FILE * output = fopen( filename, "w" );
    if ( output != NULL )
    {
        for ( size_t i = 0; i < size; ++i )
        {
            fprintf( output, "%d\n", arr[i] );
        }
        fclose( output );
    }
    else
    {
        perror( "File open failed" );
    }
}

I think this is much better than trying to figure out where your memory guesswork went wrong.

---

Edit: On second thought, I would have that function take a FILE * argument instead of a filename, which would give you the flexibility of printing to an already-opened stream (like stdout) as well, and also let you do the error handling of the fopen in a place higher up that might have additional capabilities to give useful information.

Answer 2

size is 50000, defined in #DEFINE. This is working code. But if I delete 8 multiplying to size, that is I supposed to be working, doesn't work. I got in that situation Segmentation fault: 11 Why should I allocate 8 times more memory than I need?

You are writing about this size estimates:

    char *string = (char*)calloc( (8*size+1), sizeof(char) );

But the array in use is int[] and you will write one value per line in disk as in

    sprintf( &string[ strlen(string) ], "%d\n", arr[i] );

This seems unnecessary complicated. As for the size, assume all values as INT_MIN, a.k.a. (in limits.h)

#define INT_MIN     (-2147483647 - 1)

for an 4-byte integer. So you have 11 chars. Just that. 10 digits plus one symbol for the signal. This will got you covered for any int value. Add 1 for the '\n'

But...

• Why use calloc() at all?

• Why not just use a size * 12-byte array that would fit every possible values?

• why declare a new char* to hold the value in char format instead of just using fprintf() at once?

• why void instead of just returning something like -1 for error or the number of itens written to disk in case of success?

Back to the program

If you really want to write down the array to disk in a single call to fputs(), holding the whole giant string in memory, consider that sprintf() returns the number of bytes written, so this is the value you need to use as a pointer to the output string...

If you want to use memory allocation you can do it in blocks. Considere that if all values are under 999 the 50.000 lines would have no more than 4 bytes each. But if all values are constant equal to INT_MIN you will have the max 12 bytes per line.

So you can use the return of sprintf() to update the pointer to the string, and use realloc() when needed, allocating, let's say, in blocks of a few K-bytes. (if you really want to to that write back and I can post an example)

C Example

The code below writes the file the way you tried to, and returns the total bytes written. It depends on the values of the array, anyway. The maximum is what I said, 12 bytes per line...

int fputsArray( unsigned size, int*  array , const char* filename)
{
    static char string[12 * MY_SIZE_ ] = {0};
    unsigned ix = 0; // pointer to the next char to use in string
    FILE*    output = fopen( filename, "w");
    if ( output == NULL ) return -1;
    // file is open
    for(int i = 0; i < size; i+= 1)
    {
        unsigned used = sprintf( (string + ix), "%d\n", array[i] );
        ix += used;
    } 
    fputs(string, output);
    fclose(output);
    return ix;
}

Using fprintf()

This code writes the same file, using fprintf() and is way simpler...

int fputsArray_b( unsigned size, int*  array , const char* filename)
{
    unsigned ix = 0; // bytes written
    FILE*    output = fopen( filename, "w");
    if ( output == NULL ) return -1;
    // file is open
    for(int i = 0; i < size; i+= 1)
        ix += fprintf( output, "%d\n", array[i]);
    fclose(output);
    return ix;
}

A complete test with the 2 functions

#define     MY_SIZE_ 50000
#include <limits.h>
#include <stdio.h>
#include <stdlib.h>

int fputsArray(const unsigned,int*,const char*);
int fputsArray_b(const unsigned,int*,const char*);

int main(void)
{
    int value[MY_SIZE_];
    srand(210726); // seed for today :)
    value[0] = INT_MIN; // just to test: this is the longest value
    for ( int  i=1; i<MY_SIZE_; i+=1 ) value[i] = rand();
    int used = fputsArray( MY_SIZE_, value, "test.txt");
    printf("%d bytes written to disk\n", used );

    used = fputsArray_b( MY_SIZE_, value, "test_b.txt");
    printf("%d bytes written to disk using the alternate function\n", used );
    return 0;
}


int fputsArray( unsigned size, int*  array , const char* filename)
{
    static char string[12 * MY_SIZE_ ] = {0};
    unsigned ix = 0; // pointer to the next char to use in string
    FILE*    output = fopen( filename, "w");
    if ( output == NULL ) return -1;
    // file is open
    for(int i = 0; i < size; i+= 1)
    {
        unsigned used = sprintf( (string + ix), "%d\n", array[i] );
        ix += used;
    } 
    fputs(string, output);
    fclose(output);
    return ix;
}

int fputsArray_b( unsigned size, int*  array , const char* filename)
{
    unsigned ix = 0; // bytes written
    FILE*    output = fopen( filename, "w");
    if ( output == NULL ) return -1;
    // file is open
    for(int i = 0; i < size; i+= 1)
        ix += fprintf( output, "%d\n", array[i]);
    fclose(output);
    return ix;
}

The program writes 2 identical files...