Problem with malloc in C when creating string concatenation function

Question

char *ft_concat_params(int argc, char **argv)
{
    int index;
    int aux2;
    char **newString = argv;

    aux2 = 0;


    while (argv[aux2] != '\0')
    {
    index = ft_strlen(argv[aux2]);
    newString[aux2] = (char *) malloc (index * sizeof(char *));
    ft_strcpy(newString[aux2],newString[aux2]);
    aux2++;
    }

I have this code. When I give the malloc, the function is empty. Any solution?

Answer 1

First count how much memory you will need. Then store the memory and only then start concatenating. So we don’t have to use ft_strlen twice in each string, you can use a cache.

Moreover, in the malloc you used sizeof(char *) instead of sizeof(char), and so would end up allocating much more memory than necessary. Actually, as sizeof(char) is 1 always, you do not need to use it in this case.

Also, his ft_strcpy(newString[aux2],newString[aux2]); is very strange. Putting the contents of a string in itself is not something that makes a lot of sense.

And you’re allocating the string inside the while (and nothing to release it). Since you only want to produce a single string at the end, allocate within the while won’t work.

First of all, we need #include <stdlib.h> and the implementation of ft_strcpy and ft_strlen:

char *ft_strcpy(char *dst, const char *src) {
    int i = -1;
    do {
        i++;
        dst[i] = src[i];
    } while (src[i]);
    return dst;
}

int ft_strlen(const char *src) {
    int i = 0;
    while (src[i]) i++;
    return i;
}

Here’s your revised code:

char *ft_concat_params(int argc, char **argv) {

    // Cache com os tamanhos das strings.
    int *cache = (int *) malloc(argc * sizeof(int));

    // Conta o tamanho das strings.
    int count = 1; // 1 = espaço do terminador nulo.
    for (int i = 0; i < argc; i++) {
        cache[i] = ft_strlen(argv[i]);
        count += cache[i];
    }

    // Cria a nova string que será retornada.
    char *newString = (char *) malloc(count);

    // Copia as strings para o newString.
    // Usa aux para percorrer o espaço de memória de newString.
    char *aux = newString;
    for (int i = 0; i < argc; i++) {
        ft_strcpy(aux, argv[i]);
        aux = &(aux[cache[i]]);
    }

    // Adiciona o terminador nulo.
    aux[0] = 0;

    // Não precisamos mais da cache.
    free(cache);

    // Terminamos.
    return newString;
}

To test this code (along with the #include <stdio.h>), we use that:

int main(void) {
    const char *strings1[] = {"banana", "pera", "uva"};
    char *a = ft_concat_params(3, strings1);
    printf("[%s] - %d\n", a, ft_strlen(a));
    free(a);

    const char *strings2[] = {"", "teste", "", "mais", "", "perigoso", ""};
    char *b = ft_concat_params(7, strings2);
    printf(["%s] - %d\n", b, ft_strlen(b));
    free(b);

    const char *strings3[] = {"", "", ""};
    char *c = ft_concat_params(3, strings3);
    printf("[%s] - %d\n", c, ft_strlen(c));
    free(c);

    const char *strings4[] = {};
    char *d = ft_concat_params(0, strings4);
    printf("[%s] - %d\n", d, ft_strlen(d));
    free(d);

    return 0;
}

Here’s the way out:

[bananaperauva] - 13
[testemaisperigoso] - 17
[] - 0
[] - 0

The latter two were to be empty even. They are special case tests.

See here working on ideone.

This can be implemented without using the cache or using ft_strlen more than once, if we don’t use the ft_strcpy by swapping the piece that copies the strings for this:

    // Copia as strings para o newString.
    // Usa aux para percorrer o espaço de memória de newString.
    char *aux = newString;
    for (int i = 0; i < argc; i++) {
        for (int j = 0; argv[i][j]; j++, aux++) {
            aux[0] = argv[i][j];
        }
    }

The output produced is the same.

See here working on ideone.

To know which of the approaches would perform best, it would be necessary to do very complex tests, and this also depends on how the ft_strcpy is implemented. Other very different approaches are certainly also possible.