Why am I getting segmentation fault (core dumped) for this C code?

Viewed 30

I am trying to write a simple code to solve a morse code. But I am getting segmentation fault (core dumped) although I use malloc to allocate memory. I don't understand why did I get segmentation fault. How can I solve this problem?

My code is

    const char *morse[55] = {".-", "-...", "-.-.", "-..", ".", "..-.", "--.", "....", "..", ".---", "-.-", ".-..", "--", "-.", "---", ".--.", "--.-", ".-.", "...", "-", "..-", "...-", ".--", "-..-", "-.--", "--..", "-----", ".----", "..---", "...--", "....-", ".....", "-....", "--...", "---..", "----.", ".-.-.-", "--..--", "..--..", ".----.", "-.-.--", "-..-.", "-.--.", "-.--.-", ".-...", "---...", "-.-.-.", "-...-", ".-.-.", "-....-", "..--.-", ".-..-.", "...-..-", ".--.-.", "...---..."};  
    const char *ascii[55] = {"A",  "B",    "C",    "D",   "E", "F",    "G",   "H",    "I",  "J",    "K",   "L",    "M",  "N",  "O",   "P",    "Q",    "R",   "S",   "T", "U",   "V",    "W",   "X",    "Y",    "Z",    "0",     "1",     "2",     "3",     "4",     "5",     "6",     "7",     "8",     "9",     ".",      ",",      "?",      "'",      "!",      "/",     "(",     ")",      "&",     ":",      ";",      "=",     "+",     "-",      "_",      "\"",     "$",       "@",      "SOS"};
    
    char *decode_morse(const char* morse_code)
    {
        int c1;
        char    *str;
        char    *result;
        int c2;
        int i;
        int j;
    
        i = 0;
        j = 0;
        c1 = 0;
        result = (char *)malloc(sizeof(char) * strlen(morse_code));
        str = (char *)malloc(9 * sizeof(char) +1);
        while (morse_code[c1] != '\0')
        {
            while (morse_code[c1] == '.' || morse_code[c1] == '-')
            {
                    c2 = 0;
                while (morse_code[c1] != ' ')
                {
                        str[c2] = morse_code[c1];
                        c1++;
                        c2++;
                }
                while (strcmp(str,morse[i]) != 0)
                {
                        i++;
                        if (strcmp(str,morse[i]) == 0)
                        result[j++] = ascii[i][0];
                    }
            }
                c1++;
                result[j++] = ' ';
        }
        return (result);
    }
    
    int main()
    {
        printf("%s\n", decode_morse(".... . -.--   .--- ..- -.. ."));
    }
3 Answers

Please note that I'm not an expert C developer.

I only know a bit of C++, but let me try to help. I can see that you have a few bugs in your code

See below my fix with comments

const char *morse[55] = {".-", "-...", "-.-.", "-..", ".", "..-.", "--.", "....", "..", ".---", "-.-", ".-..", "--", "-.", "---", ".--.", "--.-", ".-.", "...", "-", "..-", "...-", ".--", "-..-", "-.--", "--..", "-----", ".----", "..---", "...--", "....-", ".....", "-....", "--...", "---..", "----.", ".-.-.-", "--..--", "..--..", ".----.", "-.-.--", "-..-.", "-.--.", "-.--.-", ".-...", "---...", "-.-.-.", "-...-", ".-.-.", "-....-", "..--.-", ".-..-.", "...-..-", ".--.-.", "...---..."};  
const char *ascii[55] = {"A",  "B",    "C",    "D",   "E", "F",    "G",   "H",    "I",  "J",    "K",   "L",    "M",  "N",  "O",   "P",    "Q",    "R",   "S",   "T", "U",   "V",    "W",   "X",    "Y",    "Z",    "0",     "1",     "2",     "3",     "4",     "5",     "6",     "7",     "8",     "9",     ".",      ",",      "?",      "'",      "!",      "/",     "(",     ")",      "&",     ":",      ";",      "=",     "+",     "-",      "_",      "\"",     "$",       "@",      "SOS"};

char *decode_morse(const char* morse_code)
{
    int c1;
    char    *str;
    char    *result;
    int c2;
    int i;
    int j;


    i = 0;
    j = 0;
    c1 = 0;
    result = (char *)malloc(sizeof(char) * strlen(morse_code));
    str = (char *)malloc(9 * sizeof(char) +1);
    while (morse_code[c1] != '\0')
    {
        while (morse_code[c1] == '.' || morse_code[c1] == '-')
        {
            c2 = 0;
            while (morse_code[c1] != ' ')
            {
                    str[c2] = morse_code[c1];
                    c1++;
                    c2++;
            }
            // you need to reset the value of i here
            i = 0;

            while (strcmp(str,morse[i]) != 0)
            {
                    // there is a possibility that morse DOES NOT contain str
                    // in that case i will get out of bound and that's where you get the segmentation fault
                    // in you original case, this is probably caused by the bug below where you don't reset 'str'
                    // resulting in str being something that is not even valid morse in the first place
                    i++;
                    if (strcmp(str,morse[i]) == 0)
                    result[j++] = *ascii[i];
            }
            // you are assigning the char being read to the variable 'str' without ever resetting 'str'
            // imagine if you get .... and .- then your str will be .-.. for that character 
            // since you never remove the 3rd and 4th char from previous loop
            for (int a = 0; a < 10; a++) {
                str[a] = 0;
            }
        }
       
        c1++;
        result[j++] = ' ';
    }
    return (result);
}

int main()
{
    printf("%s\n", decode_morse(".... . -.--   .--- ..- -.. ."));
}

The value of i increments to 55 and you try to index out of bounds. Using while loops without safeguards like that is pretty dangerous, consider turning them to for loops instead.

grandia's answer is good and I agree with his reasoning. str is getting corrupted and it is not matching any of the strings in the array morse. That is why that while loop goes out of bounds.

I have one suggestion though, instead of resetting str, it is better to append a null character ('\0') at the end of its current set of characters, immediately after the while loop.

c2 = 0;
while (morse_code[c1] != ' ') {
    str[c2] = morse_code[c1];
    c1++;
    c2++;
}
str[c2] = '\0';

This ensures that all the characters after the null character are ignored. str need not be reset completely.

Also, note that when str is allocated we assume that str will have maximum 9 characters, apart from the null character. We make use of that now.

Also, this will ensure that the strcmp that comes later will always work properly. This includes the first time strcmp is called, which is not guaranteed if str is only going to be reset later.

Related