This is a function that reads until EOF and reallocs as it goes.

char *
read_to_end(int fd, int *bread)
{
    ssize_t size, nread, rc;
    char *buf, *tmp;

    nread = 0;
    size = MINLEN;
    buf = malloc(size);

    while ((rc = read(fd, buf + nread, size - nread))) {
        if (rc < 0) {
            if (errno == EINTR)
                continue;

            goto error;
        }

        nread += rc;

        /* See if we need to expand. */
        if (size - nread < MINLEN) {
            size *= 2;

            /* Make sure realloc doesn't fail. */
            if (!(tmp = realloc(buf, size)))
                goto error;

            buf = tmp;
        }
    }

    /* Shrink if necessary. */
    if (size != nread && !(tmp = realloc(buf, nread)))
        goto error;

    buf = tmp;
    *bread = nread;
    return buf;

error:
    free(buf);
    return NULL;
}

Do you have any improvement suggestions?

  • You can use fseek() and ftell() to get the size if this is a file. – Martin York Feb 25 '12 at 21:33
  • Without knowing more about which goal you want to reach is very difficult to give you a direction. But IMO it is never a good practice to bufferize a whole copy of a file into memory. – greydet Mar 13 '12 at 21:23

Seeing this is 'unanswered' here's my tuppence worth:

If you are reading a file I suggest you get the file size first, eg using fstat(), allocate a buffer of the correct size and read into that.

Alternatively, if the file is big and you have OS support, consider mapping the file into memory using mmap(). This maps the file into virtual memory but does not read it immediately. As you access the memory into which the file is mapped the file is read into that address space for you (in the background) by the OS.

On your code, I have a few comments:

  • If you have a stderr, perhaps add a call to perror() on error.

  • It looks as if you always re-alloc, even if the initial buffer is big enough.

  • If zero bytes are read, you will realloc(buf, 0). realloc man page says this is ok.

  • Add a bracket around size != nread - I always have to check operator precedence in statements like this and I guess I am not alone. Brackets save the reader the trouble.

  • I would prefer *bread to be set to 0 on failure. Should bread be size_t ?

I would refrain of using GOTO in pure C code. It's not very common...

Also, when you return the buf pointer to the calling function, that function is going to need to actively manage that piece of memory (It becomes in charge of the "free"-ing process).

  • 2
    Using goto to jump to a common cleanup block is actually quite common, and a good idea. – Graham Borland Mar 12 '12 at 15:02
  • I wasn't aware of that. Either way, isn't goto a poor man's exception handling? – Evert Mar 12 '12 at 15:26
  • Generally speaking we try to avoid using GOTOs because it can make code less readable and hard to maintain. But there is some cases (like error handling) where it is really convenient to use GOTOs instead of levels of conditional statement. – greydet Mar 13 '12 at 21:31
  • @Evert Poor man’s exception handling is exactly what it is here. And aptly so, since C doesn’t provide exceptions. This usage of goto is about the only one I’d condone. – Konrad Rudolph Mar 31 '12 at 21:44

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.