1

I'm trying to concat two strings, supposing the "dest" string hasn't enough space to add another one, so I'm using dynamic arrays to solve it.

The problem is a mremap_chunk error when trying to compile the code.

I don't know what I'm missing since the realloc call has all the right params place in.

Error: 

malloc.c:2869: mremap_chunk: Assertion `((size + offset) & (GLRO (dl_pagesize) - 1)) == 0' failed. 
Aborted (core dumped)

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

char *strcatt(char *s1, char *s2)
{
    int a = strlen(s1);
    int b = strlen(s2);
    int i, size_ab = a+b;

    s1 = (char *) realloc (s1, size_ab*sizeof(char));

    for(i=0; i<b; i++) {
        s1[i+a]=s2[i];
    }

    s1[size_ab]='\0';

    return s1;
}


int main()
{
    char s1[]="12345";
    char s2[]="qwerty";

    strcatt(s1,s2);
    printf("%s\n", s1);

    return 0;
}
prtpereira
  • 11
  • 1
  • 3

3 Answers3

4

First, you are treating non-heap memory as heap memory, don't do that.

Second you're not including space for the terminator in the calculation.

Here are some more points:

  1. Don't name functions starting with str, that's a reserved name space.
  2. Buffer sizes should be size_t, not int.
  3. Don't cast the return value of malloc() in C.
  4. Use memcpy() to copy blocks of memory when you know the size.
  5. The "right hand side" strings should be const.
  6. Deal with the possibility of allocation error.
  7. I consider it bad practice to scale by sizeof (char), that's always 1.

Here's how I would write it, assuming the same logic:

char * my_strcatt(char *s1, const char *s2)
{
    const size_t a = strlen(s1);
    const size_t b = strlen(s2);
    const size_t size_ab = a + b + 1;

    s1 = realloc(s1, size_ab);

    memcpy(s1 + a, s2, b + 1);

    return s1;
}
Naeio
  • 1,112
  • 7
  • 21
unwind
  • 391,730
  • 64
  • 469
  • 606
3

You can not realloc or free a memory that is not allocated with a call to malloc or is not NULL.

From section 7.22.3.5. The realloc function in C11 draft

The realloc function deallocates the old object pointed to by ptr and returns a pointer to a new object that has the size specified by size. The contents of the new object shall be the same as that of the old object prior to deallocation, up to the lesser of the new and old sizes. Any bytes in the new object beyond the size of the old object have indeterminate values.

So, s1 = (char *) realloc (s1, size_ab*sizeof(char)); is plainly wrong for your inputs (automatic arrays), never do that.

And then there are many more problems which can be fixed with some help from a debugger.

Mohit Jain
  • 30,259
  • 8
  • 73
  • 100
2

The clang debugger gives a very clear error description:

malloc:  error for object 0x7fff6fbb16d6: pointer being realloc'd was not allocated
 set a breakpoint in malloc_error_break to debug

Both of your arrays are initialized as string literals. Further on, your function tries to modify a string literal by reallocing it, which is wrong by C standard because you can't reallocate what you haven't allocated, and then copying the members of the second string literal to the "object" you intended to modify by misusing realloc() on a string literal.

The code would work if you had dynamically defined a third string in which you would have summed the contents of both:

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

char *mystrcatt(char *s1, char *s2)
{
    int a = strlen(s1);
    int b = strlen(s2);
    int i, size_ab = a+b;

    char *s3 = malloc (size_ab*sizeof(char)); //sizeof(char) is always 1

    for(i=0; i<a; i++) { //inefficient
        (s3[i])=s1[i];
    }    

    for(i=0; i<b; i++) { //inefficient
        (s3[i+a])=s2[i];
    }

    s3[size_ab]='\0';

    return s3;
}


int main()
{
    char s1[]="12345";
    char s2[]="qwerty";
    char *s3 = mystrcatt(s1,s2);    
    printf("%s\n", s3);
    free(s3);
    return 0;
}

Please, also note that you don't cast the return of malloc() in C.

Community
  • 1
  • 1
user3078414
  • 1,942
  • 2
  • 16
  • 24