current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

Very useful operation that hasn't been merged into a function.. until now. This is supposed to insert a substring into a previously d.allocated source string.

void strapp (char *source_offset, int position, char *appendage)
{
    size_t appendage_size = strlen(appendage) + 1;

    char copy[strlen(source_offset)];
    strcpy(copy, source_offset);

    source_offset = realloc(source_offset, strlen(source_offset) + appendage_size);
    memcpy(&source_offset[position], appendage, strlen(appendage));
    sprintf(source_offset + (position + strlen(appendage)), &copy[position]);
}

USAGE:

int main(void)
{
    char *str1 = malloc(11 + 1);

    sprintf(str1, "Hello World");

    strapp(str1, 5, " Horrible");

    printf(str1);
    free(str1);

    return 0;
}

Produces the output:

Hello Horrible World

share|improve this question

2 Answers 2

up vote 3 down vote accepted

This is more of a string-inserting function than a string-appending function, and should be renamed as such.

When calling realloc(…), the string could very well move to a different memory location. The caller of strapp() has no way of knowing where the new string is, though. Furthermore, if the string got moved, and the caller continues to use the string as if were still at source_offset, then that's a memory-use-after-free bug.

There are two possible remedies:

/**
 * s points to the address of the old and new string.
 */
string_insert(char **s, int insertion_pt, const char *to_insert)

or

/**
 * Returns the location of the new string, which may or may not be at
 * the original s.  Callers must assume that the string at s may be
 * invalid after this call.
 */
char *string_insert(char *s, int insertion_pt, const char *to_insert)
share|improve this answer
up vote 1 down vote 
    sprintf(source_offset + (position + strlen(appendage)), &copy[position]);

may produce an undefined behavior if a trailing part of the source string has embedded % characters. Either use an explicit format string 

    sprintf(source_offset + (position + strlen(appendage)), "%s", &copy[position]);

or memcpy it.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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