Search code examples
cmemory-managementdynamic-memory-allocation

Manipulating dynamic arrays in C


I am trying to solve StringMerge (PP0504B) problem from SPOJ (PL). Basically the problem is to write a function string_merge(char *a, char *b) that returns a pointer to an char array with string created from char arrays with subsequent chars chosen alternately (length of the array is the length of the shorter array provided as an argument).

The program I've created works well with test cases but it fails when I post it to SPOJ's judge. I'm posting my code here, as I believe it the problem is related to memory allocation (I'm still learning this part of C) - could you take a look at my code?

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

#define T_SIZE 1001

char* string_merge(char *a, char *b);

char* string_merge(char *a, char *b) {
    int alen = strlen(a); int blen = strlen(b);
    int len  = (alen <= blen) ? alen : blen;
    int i,j;

    char *new_array = malloc (sizeof (char) * (len));
    new_array[len] = '\0';
    for(j=0,i=0;i<len;i++) {
            new_array[j++] = a[i];
            new_array[j++] = b[i];
    }
    return new_array;
}

int main() {
    int n,c; scanf("%d", &n);
    char word_a[T_SIZE];
    char word_b[T_SIZE];
    while(n--) {
        scanf("%s %s", word_a, word_b);
        char *x = string_merge(word_a, word_b);
        printf("%s",x);
        printf("\n");
        memset(word_a, 0, T_SIZE);
        memset(word_b, 0, T_SIZE);
        memset(x,0,T_SIZE);
    }
  return 0;
}

Note: I'm compiling it with -std=c99 flag.


Solution

  • Off-by-one.

    char *new_array = malloc (sizeof (char) * (len));
    new_array[len] = '\0';
    

    You're writing past the bounds of new_array. You must allocate space for len + 1 bytes:

    char *new_array = malloc(len + 1);
    

    Also, sizeof(char) is always 1, so spelling it out is superfluous, so are the parenthesis around len.

    Woot, further errors!

    So then you keep going and increment j twice within each iteration of the for loop. So essentially you end up writing (approximately) twice as many characters as you allocated space for.

    Also, you're leaking memory by not free()ing the return value of string_merge() after use.

    Furthermore, I don't see what the memsets are for, also I suggest you use fgets() and strtok_r() for getting the two words instead of scanf() (which doesn't do what you think it does).