Replace char array in other char array

Question

I have a char array and I want to replace 1* or 2* for myDirectory1 or myDirectory2

Ex: C:/1*/file.txt -> C:/MDirectory1/file.txt
Ex: C:/2*/file.txt -> C:/MDirectory2/file.txt

My code seems to works but I do not understand some things:

Is okay to initialize a dynamic char array like this?

char *cad = (char*)malloc(sizeof(char) * 15);
if (!cad) exit(1);

Is okay to use realloc when I want to insert the char array inside the other?

realloc(*cad, sizeRep + strlen(auxi) + 1);

My code:

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

char *replaceString(char **cad) {
    int sizeRep;
    char *auxi = *cad, *plec;
    char replace[500];
    plec = strchr(*cad, '*');
    switch (*(plec - 1)) {
      case '1':
        strcpy(replace, "myDirectory1");
        break;
      case '2':
        strcpy(replace, "MyDirectory2");
    }
    sizeRep = strlen(replace);
    realloc(*cad, sizeRep + strlen(auxi) + 1);
    memmove(plec + strlen(replace) - 1, plec + 1, strlen(plec));
    memmove(plec - 1, replace, sizeRep);
    return auxi;
}

int main() {
    char *cad = (char*)malloc(sizeof(char) * 15);
    if (!cad) exit(1);
    strcpy(cad, "C:/2*/file.txt");
    printf("%s", replaceString(&cad));
    return 0;
}

Possible duplicate of [how to replace substring in c?](https://stackoverflow.com/questions/3659694/how-to-replace-substring-in-c) — Fabien, Jul 13 '17 at 00:08
`realloc(*cad,sizeRep+strlen(auxi)+1);` --> `*cad = realloc(*cad, sizeRep+strlen(auxi)+1);` and You should think that `auxi` and `plec` become invalid after `realloc`. — BLUEPIXY, Jul 13 '17 at 00:09
Do You talk about `if (!cad) exit(1);` ? It is more helpful to output a message explaining the cause. — BLUEPIXY, Jul 14 '17 at 20:14
I want to use if(!cad)return NULL; after realloc to prevent auxi and plec from becoming invalid. — Juan Olivadese, Jul 14 '17 at 20:39
If `realloc` succeeds (in most cases) the previous content is copied to the newly reserved area. So pointers pointing to some of the old areas will be invalid. — BLUEPIXY, Jul 15 '17 at 06:16
If `realloc` fails, `auxi` can be used, but after that event processing can not continue. It is reasonable to return `NULL` if you want error handling to be left to the caller. — BLUEPIXY, Jul 15 '17 at 06:22

score 0 · Answer 1 · answered Jul 13 '17 at 05:11

In your example there is no major issues that I can see. But I like to add come comments.

After calling "realloc" function the value of *cad could ne invalid if there in no enough memory to allocate. So be conscious about it as BLUEPIXY suggested in his comment.
The *cad pointer use differently in in various location. In the main it is a pointer. But in the replaceString it is a double pointer. Pragmatically there is no issue since the scope is different. But it is preferable to avoid such usage.

chqrlie · Accepted Answer · 2017-07-13T10:48:42.340

Your code has multiple errors:

you do not check if * was found and if it was found at an offset greater than 0.
you do not copy the null terminator.
your computations are off by one.
you do not store the return value of realloc()
you access a location in the string that was reallocated, this has potential undefined behavior if the string was moved. Use the offset instead.
the replacement for 2* has a different case from that of 1*.

Here is a corrected version:

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

char *replaceString(char **cad) {
    char *auxi = *cad;
    size_t len = strlen(auxi);
    char *plec = strchr(auxi, '*');

    if (plec && plec > auxi) {
        size_t offset = plec - auxi - 1;
        size_t sizeRep;
        const char *replace = NULL;

        switch (auxi[offset]) {
          case '1':
            replace = "myDirectory1";
            break;
          case '2':
            replace = "myDirectory2";
            break;
          default:
            return auxi;
        }
        sizeRep = strlen(replace);
        // reallocate auxi: -2 for the characters removed, +1 for the null termnator
        auxi = realloc(auxi, len - 2 + sizeRep + 1);
        if (auxi == NULL)
            return NULL;
        memmove(auxi + offset + sizeRep, auxi + offset + 2, len - offset - 2 + 1);
        memmove(auxi + offset, replace, sizeRep);
        *cad = auxi;
    }
    return auxi;
}

int main(void) {
    char *cad = strdup("C:/2*/file.txt");
    if (!cad)
        exit(1);
    printf("%s\n", replaceString(&cad));
    free(str);
    return 0;
}

realloc() is a notorious source of problems, leading to undefined behavior and memory leaks. You might want to change your function to use a simpler API, always allocate the return value, without side effects on the arguments:

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

char *replaceString(const char *src) {
    size_t size = strlen(src) + 1;
    size_t offset = 0, matchlen = 0, replace_len = 0;
    const char *replacement = "";
    const char *p;
    char *dest;

    if ((p = strstr(src, "1*")) != NULL) {
        offset = p - src;
        match_len = 2;
        replacement = "myDirectory1";
        replacement_len = strlen(replacement);
    } else
    if ((p = strstr(src, "2*")) != NULL) {
        offset = p - src;
        match_len = 2;
        replacement = "myDirectory2";
        replacement_len = strlen(replacement);
    }
    dest = malloc(size - match_len + replacement_len);
    if (dest) {
        memcpy(dest, src, offset);
        memcpy(dest + offset, replacement, replacement_len);
        memcpy(dest + offset + replacement_len,
               src + offset + match_len, size - offset - match_len);
    }
    return dest;
}

int main(void) {
    char *str = replaceString("C:/2*/file.txt");
    if (!str)
        exit(1);
    printf("%s\n", str);
    free(str);
    return 0;
}

Replace char array in other char array

2 Answers2