C-string appender for buffers and strings Is this C appender safe, and reasonably optimal?

#define REALLOC_BUFFER_SIZE 4096

char *append(char *orig_str, size_t *orig_str_size, const char *append_str) {
    // no action required if appending an empty string
    if (append_str[0] == 0) {
        return orig_str;
    }

    // does the current char* have enough space?
    size_t req_space = strlen(orig_str) + strlen(append_str) + 1; // +1 for NULL byte

    // resize if the required space is greater than the original buffer size.
    if (req_space &gt; *orig_str_size) {
        // prepare the variable with the amount of space we want
        size_t allocate = *orig_str_size + req_space + REALLOC_BUFFER_SIZE;

        char *new_str = (char *)realloc(orig_str, allocate);
        // resize success.. 
        if (new_str == NULL) {
            fprintf(stderr, "Couldn't reallocate memory\n");
            return NULL;
        }

        // the realloc didn't fail, here is our new pointer
        orig_str = new_str;

        // overwrite the free space with NULL's
        memset(orig_str + strlen(orig_str), 0, req_space);

        // copy the appending text to the string
        strcpy(orig_str + strlen(orig_str), append_str);

        // modify the original size
        *orig_str_size = allocate;

    } else {
        // append the string
        strcpy(orig_str + strlen(orig_str), append_str);
    }

    return orig_str;
}


Example usage:

char *buffer;
size_t buffer_size = 20;
int i;

buffer = (char *)malloc(sizeof(char) * buffer_size);
strcpy(buffer, "1234567890123456789"); // fill some data prior.. 

// original string
for (i=1; i &lt; 100; ++i) {
    buffer = append(buffer, &amp;buffer_size, "AABBCC");
}

free(x);


Latest changes: GitHub
In addition to what the other answers say and I generally agree with:

Don't call strlen more than once.  That's just a waste.  Count the length of orig_str and append_str once at the beginning of the function and store them away in const variables.  That will also make your initial check whether append_str is empty look cleaner:

const size_t append_len = strlen(append_str);
if (!append_len)
    return orig_str;
const size_t orig_len = strlen(orig_str);


Moving the strlen(orig_str) above the conditional would look cleaner in my eyes but it might be a pessimization if append_str is often empty.

Of course, the most efficient way would be if you could pass orig_len as an additional input parameter.  Or you could use a

struct string_buffer
{
  size_t length;
  size_t capacity;
  char * data;
};


that encapsulates all the required info and change your function's signature to

ssize_t
append_string(struct string_buffer * buffer, const char * appendix);


where the function could return the number of bytes successfully appended or –1 on error.

My second remark would be that you are growing your buffer by a constant amount each time you reallocate.  It should really grow exponentially unless you really know that linear growth will be preferable in your specific use-case, which I find hard to imagine how that could be.

Concerning your comments: there is no NULL byte.  NULL is a macro constant for the null pointer.  The null byte is abbreviated as NUL in the ASCII character set.  I would also find it helpful to have your function's semantics exactly documented in a comment.  For example, should the user free the buffer if it fails and returns NULL?  Will the existing data be modified in that case?  I can try answering those questions by reading your implementation but that doesn't allow me to distinguish a bug in your code from a different valid design decision.