String appending function doesn't work

Question

I tried to create a function that easily appends two strings. The source string has to be previously allocated dynamically. I used my knowledge but this resulted in ub and any sort of leaks.

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

    source_offset = realloc(source_offset, strlen(source_offset) + appendage_size);
    sprintf( &source_offset[position], "%s%s", appendage, source_offset + (position + appendage_size) );
}

What am I doing wrong?

Because this is a function I created for an engine. This is not a coding from scratch.. and strcat isn't included because of a mysterious reason. — Alan Salios
– Alan Salios, Commented Feb 2, 2015 at 6:18
How is the caller supposed to know where did his string go if realloc decides to move it elsewhere? Also, you fell into the usual x = realloc(x, n) antipattern. — Matteo Italia
– Matteo Italia, Commented Feb 2, 2015 at 6:23
What is position supposed to be? Regardless of what it is, the sprintf args seem non-sensical -- what are they trying to do? — Chris Dodd
– Chris Dodd, Commented Feb 2, 2015 at 6:29
@MatteoItalia x = realloc(x, n); is fine so long as you plan to abort the process on failure. The pass-by-value error is orthogonal (it'd still occur if using the better realloc pattern) — M.M
– M.M, Commented Feb 2, 2015 at 6:41

Jean-Baptiste Yunès · Accepted Answer · 2015-02-02 08:57:25Z

source_offset + (position + appendage_size) is somehow strange. It seems that you tried to catenate the second string with a substring of the first copying the result in the first string.

source_offset + (position + appendage_size) is the suffix of the source string starting at offset position+appendage_size which is a non-sense as it is past the end of the source string...

May be you wanted something like this? If you want to catenate the two string then the following is correct:

size_t appendage_size = strlen(appendage);
source_offset = realloc(source_offset, position + appendage_size + 1);
sprintf( &source_offset[position], "%s", appendage );

Which appends appendage to source_offset starting at position.

Now if you want to insert appendage in the middle this can be a little more tricky:

size_t appendage_size = strlen(appendage);
char *result = malloc(strlen(source_offset) + appendage_size + 1);
char cut = source_offset[position];
source_offset[position] = '\0';
sprintf( result, "%s%s%c%s", source_offset,appendage,cut,source_offset+position+1);
// do hat you want with result

Beware that realloc may change the base address of the initial memory, so you can't do things like this as the parameter value source_offset will be changed only locally.

Gopi · Accepted Answer · 2015-02-02 06:20:15Z

0

size_t appendage_size = strlen(appendage) + 1; /* To hold a \0 character */

The new array allocated should be able to hold both the null characters in each string

strlen(string) + 1

answered Feb 2, 2015 at 6:20

Gopi

19.9k4 gold badges28 silver badges37 bronze badges

2 Comments

Alan Salios Over a year ago

It still doesn't work. Usage: char *str1 = malloc(11 + 1); strcpy(str1, "Hello World"); strapp(str1, 4, " Horrible "); printf(str1); free(str1);

Gopi Over a year ago

@AlanSalios In order to append you need to realloc() to hold the new string

Sridhar Nagarajan · Accepted Answer · 2015-02-02 06:36:39Z

actually your coes does not append string B to string A, it does append string B at position n of string A along with string A, with some out of memory offset of String A

char *
strapp(char *dest, const char *src)
{
    size_t i,j;
    for (i = 0; dest[i] != '\0'; i++)
        ;
    for (j = 0; src[j] != '\0'; j++)
        dest[i+j] = src[j];
    dest[i+j] = '\0';
    return dest;
}

the above code appends string B to String A and returns pointer to string a A;

char* strapp (char *source_offset, const char *appendage, int position)
{
    char* temp = NULL:
    size_t appendage_size = strlen(appendage);
    if (( temp = realloc(source_offset, strlen(source_offset) + appendage_size + 1)) == NULL )
        return NULL;
    else
        source_offset = temp;
    sprintf( &source_offset[position], "%s", appendage);
    return source_offset;
}

Jonathan Leffler · Accepted Answer · 2015-02-02 06:37:35Z

0

If you want to fix your function try this:

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

    *source_offset = (char*)realloc(*source_offset, strlen(*source_offset) + appendage_size);
    sprintf( *source_offset + position, "%s%s", appendage, *source_offset + (position + appendage_size) );
}

And call this like strapp(&str, 10, "INSERTION"); // &str - pointer to pointer.

The problem was in the following: when you sent pointer to function you are able to change data pointed by this pointer, but cannot change the pointer, so memory allocation made inside function do mot change address of initial string.

edited Feb 2, 2015 at 6:37

Jonathan Leffler

760k145 gold badges961 silver badges1.3k bronze badges

answered Feb 2, 2015 at 6:29

VolAnd

6,4173 gold badges29 silver badges44 bronze badges

1 Comment

Alan Salios Over a year ago

Like that it doesn't print out what is after the appended substring. At least it isn't a leak like it was before..

Alan Salios · Accepted Answer · 2015-02-02 06:42:01Z

0

Since I was asked to provide an example of function usage and respectively the output. I converted this code to something compilable to mingw and it works like expected.

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

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

    source_offset = realloc(source_offset, strlen(source_offset) + appendage_size);
    sprintf( &source_offset[position], "%s%s", appendage, source_offset + (position + appendage_size) );
}

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

    sprintf(str1, "Hello World");

    strapp(str1, 4, " Horrible ");

    printf(str1);
    free(str1);

    return 0;
}

Output: Hell Horrible

answered Feb 2, 2015 at 6:42

Alan Salios

2432 silver badges10 bronze badges

2 Comments

Alan Salios Over a year ago

Since when malloc has two arguments?

Jean-Baptiste Yunès Over a year ago

Ok, I'm pretty sure you can correct it by yourself, albeit I made the correction.

Alan Salios · Accepted Answer · 2015-02-02 07:11:09Z

Partial fix of the problem I found while investigating. If I add a substring to a location of the source string like that:

sprintf(&source[4], "new");

the source string will be terminated by the "new" and the rest of the string will not be shown. But if I declare the substring "new" and use memcpy(&str1[4], appendage, 3); it does the job.

There, this is the working function:

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

    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]);
}

Collectives™ on Stack Overflow

String appending function doesn't work

6 Answers 6

Comments

2 Comments

Comments

1 Comment

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

Comments

2 Comments

Comments

1 Comment

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related