Segmentation Fault:11 in C

Question

I am writing a program in C that replaces a number in a char* called "template" with a string, but I continually get a Segmentation Fault: 11 error.

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

char *rep_str(const char *s, const char *old, const char *new1){
    char *ret;
    int i, count = 0;
    int newlen = strlen(new1);
    int oldlen = strlen(old);

    for (i = 0; s[i] != '\0'; i++){
        if (strstr(&s[i], old) == &s[i]){
            count++;
            i += oldlen - 1;
        }
    }
    ret = (char*)malloc(i + count * (newlen - oldlen));
    if (ret == NULL)
        exit(EXIT_FAILURE);
    i = 0;
    while (*s){
        if (strstr(s, old) == s){ //compare the substring with the newstring
            strcpy(&ret[i], new1);
            i += newlen; //adding newlength to the new string
            s += oldlen;//adding the same old length the old string
        } else {
        ret[i++] = *s++;
        }
    }
    ret[i] = '\0';

    return ret;
}

char* madlib_by_numbers(char* temp, int word_count, char* words[]){
    char* numbers[] = {"0", "1", "2", "3", "4", "5", "6", "7", "8", "9"};
    int tempSize = strlen(temp);

    for (int i = 0; i < tempSize; i++){
        if (isdigit(temp[i])){
            for (int j = 0; j < (sizeof(numbers) / sizeof(char*)); j++){
                temp = rep_str(temp, numbers[j], words[j]); //it makes it to this line, but never gets to assert()
            }
        }
    }

    return temp;
}

int main() {
  char* temp1 = "The 1 0 likes to 2 in the moonlight.";
  char* words[] = {"git", "brilliant", "swim"};
  char* result = "The brilliant git likes to swim in the moonlight.";
  int stringLength = strlen(result);

  char* test = madlib_by_numbers(temp1, 3, words);
  assert(strncmp(test, result, stringLength) == 0);
  free(test);

  return 0;
}

and when I run the debugger, it simply says: Segmentation Fault: 11

What i just want to understand is where the segmentation fault error is coming from, I have the suspicion one of my loops is running too many times.

Answer 1

There are a few issue with your code. However, the direct answer to your question is in this loop:

for (int j = 0; j < (sizeof(numbers) / sizeof(char*)); j++){
        temp = rep_str(temp, numbers[j], words[j]);
}

You are calling rep_str for every digit while you mean call rep_str only if the digit in temp matches the corresponding digit in numbers. So add this conditional if(strcmp(temp,numbers[j]) == 0) right before the line temp=.... Then it'll solve your current problem.

The segfault is caused because there are only three elements in the words array. Your old loop indexes from 0 to 9 and fails when j=3, out of bound.

Also, delete the free() at the end of your program. test was never allocated and will cause a core dump.

Answer 2

ret = (char*)malloc(i + count * (newlen - oldlen));

There are a few problems with this line of code.

• For a start, don't cast malloc (or any void * that you're assigning to a variable of different pointer type, or vice versa).
• If you intended to allocate space to store a string, where's the string-terminating '\0' going to go? You need to realise that for an empty old string, this will be malloc(0) and zero bytes is not enough to store an empty string.
• There's also a problem if you expect that old may be a substring of new (for example, you're replacing "0" with "hell0"). You'll need to tweak your algorithm to handle this problem. I'll leave that as a challenge for you to attempt :)

---

for (int i = 0; i < tempSize; i++){
    if (isdigit(temp[i])){
        for (int j = 0; j < (sizeof(numbers) / sizeof(char*)); j++){
            temp = rep_str(temp, numbers[j], words[j]); //it makes it to this line, but never gets to assert()
        }
    }
}

users previous answer highlighted this code correctly, but not for the right reason... and so the solution he/she presented is wrong.

isdigit(temp[i]) may also cause segfaults for some inputs. I recommend using isdigit((unsigned char) temp[i]) instead, in this case.

It's not valid to access words[j] where word_count is 3 and j is greater or equal to 3; you're accessing that array out of bounds.

You also need to be careful to free any memory you *alloc (while simultaneously not freeing memory that you don't *alloc). Forgetting to do the former won't cause crashes, but your program won't run happily; it'll use heaps of memory.

Consider something like this, instead:

temp = strdup(temp);
if (temp == NULL) {
    exit(EXIT_FAILURE);
}
for (int i = 0; i < tempSize; i++){
    if (isdigit((unsigned char) temp[i])){
        for (int i = min(word_count, sizeof(numbers) / sizeof(char*)), j = 0; j < i; j++){
            char *new = rep_str(temp, numbers[j], words[j]);
            free(temp);
            temp = new;
        }
    }
}