Alex Rivera | Logout

Reading all content from a text file - C

Asked 2010-08-01T06:54:26.880
11

I am trying to read all content from a text file. Here is the code which I wrote. 

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

#define PAGE_SIZE 1024

static char *readcontent(const char *filename)
{
    char *fcontent = NULL, c;
    int index = 0, pagenum = 1;
    FILE *fp;
    fp = fopen(filename, "r");

    if(fp) {
        while((c = getc(fp)) != EOF) {
            if(!fcontent || index == PAGE_SIZE) {
                fcontent = (char*) realloc(fcontent, PAGE_SIZE * pagenum + 1);
                ++pagenum;
            }
            fcontent[index++] = c;
        }
        fcontent[index] = '\0';
        fclose(fp);
    }
    return fcontent;
}

static void freecontent(char *content)
{
    if(content) {
        free(content);
        content = NULL;
    }
}

This is the usage

int main(int argc, char **argv)
{
    char *content;
    content = readcontent("filename.txt");
    printf("File content : %s\n", content);
    fflush(stdout);
    freecontent(content);
    return 0;
}

Since I am new to C, I wonder whether this code looks perfect? Do you see any problems/improvements?

Compiler used : GCC. But this code is expected to be cross platform.

Any help would be appreciated.

Edit

Here is the updated code with fread and ftell.

static char *readcontent(const char *filename)
{
    char *fcontent = NULL;
    int fsize = 0;
    FILE *fp;

    fp = fopen(filename, "r");
    if(fp) {
        fseek(fp, 0, SEEK_END);
        fsize = ftell(fp);
        rewind(fp);

        fcontent = (char*) malloc(sizeof(char) * fsize);
        fread(fcontent, 1, fsize, fp);

        fclose(fp);
    }
    return fcontent;
}

I am wondering what will be the relative complexity of this function?

c file-io dynamic-memory-allocation
Edit
Report

1 Answer

1

This is from a quick reading, so I might have missed a few issues.

First, a = realloc(a, ...); is wrong. If realloc() fails, it returns NULL, but doesn't free the original memory. Since you reassign to a, the original memory is lost (i.e., it is a memory leak). The right way to do this is to do: tmp = realloc(a, ...); if (tmp) a = tmp; etc.

Second, about determining the file size using fseek(fp, 0, SEEK_END);, note that this may or may not work. If the file is not random-access (such as stdin), you won't be able to go back to the beginning to read it. Also, fseek() followed by ftell() may not give a meaningful result for binary files. And for text files, it may not give you the right number of characters that can be read. There is some useful information on this topic on comp.lang.c FAQ question 19.2.

Also, in your original code, you don't set index to 0 when it equals PAGESIZE, so if your file length is greater than 2*PAGESIZE, you will overwrite the buffer.

Your freecontent() function:

static void freecontent(char *content)
{
    if(content) {
        free(content);
        content = NULL;
    }
}

is useless. It only sets a copy of content to NULL. It is just like if you wrote a function setzero like this:

void setzero(int i) { i = 0; }

A much better idea is to keep track of memory yourself and not free anything more or less than needed.

You shouldn't cast the return value of malloc() or realloc() in C, since a void * is implicitly converted to any other object pointer type in C.

Hope that helps.

answered 2010-08-01T07:39:08.613

Your Answer