Question

Does this implementation make any sense to you? I'm trying to write a function that will concatenate two strings and can be called as appendstr(&dest, "xyz"); I'm not sure at all if is a good practice what I'm doing here, reallocating the space for newptr over origptr and then free it and make it equal to newptr.

void *appendstr(char **origptr, const char *strdata)
{
    size_t len1 = strlen(*origptr);
    size_t len2 = strlen(strdata);
    char *newptr = realloc(*origptr, len1 + len2 + 1);
    if (newptr != NULL)
    {
        memcpy(newptr + len1, strdata, len2 + 1);
        free(*origptr);
        *origptr = newptr;
    }
    return newptr;
}

All I'm trying to do is to not change anything in *origptr until I'm sure that there is no problem with the memory allocation and only then, do the concatenation. Also, another concern is if I'm allocating exactly the amount of memory that I need.

Was it helpful?

Solution

This should be enough. It's just your code without the free()

void *appendstr(char **origptr, const char *strdata)
{
    size_t len1 = strlen(*origptr);
    size_t len2 = strlen(strdata);
    char *newptr = realloc(*origptr, len1 + len2 + 1);
    if (newptr != NULL)
    {
        memcpy(newptr + len1, strdata, len2 + 1);
        *origptr = newptr;
    }
    return newptr;
}

Returns NULL if it couldn't concatenate the strings, and *origptr is not changed. Otherwise, it returns the (possibly new) allocated pointer with data concatenated. *origptr then will have the same value as the returned value.

memcpy() instead of strcat() because strcat() needs to know where the string ends by calling strlen() internally (or doing its own version of strlen() ). As anyway you call strlen() because you need the length of the string to calculate the amount of memory to allocate, you can use memcpy() to directly copy the second string right after the first one, skipping an implicit second call to strlen() if were using strcat().

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top