What's causing a segmentation fault in my code and what I should do to fix it?

Question

I've been getting segmentation fault and I don't know why. I have a feeling that it's caused by the code under if(isPalindrome(str_array[i]) == 0) but I don't know which one and what to do about it.

P.S. I'm still a student so I would appreciate it if the suggestions do not go far from the level of code that I have here.

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

int isPalindrome(char check[]);

int main() {
    int array_size = 5, i;
    char *str_array[array_size], *palindrome_array[array_size], str_length[100];

    for(i = 0; i < array_size; i++) {
        printf("Enter word #%d: ", i+1);
        scanf("%[^\n]%*c", str_length); //"%[^\n]%*c : pattern matching - allows inputs to have spaces

        str_array[i] = (char*) malloc(sizeof(char) * strlen(str_length));
        strcpy(str_array[i],str_length); 

        if(strcmp(str_array[i],"exit") == 0) {
            break;
        }
    
        if(isPalindrome(str_array[i]) == 0) {
            palindrome_array[i] = (char*) malloc(sizeof(char) * strlen(str_length));
            strcpy(palindrome_array[i], str_length); 
            printf("'%s' is a palindrome \n", palindrome_array[i]);
        } else printf("'%s' is NOT a palindrome \n", str_array[i]);
    }


    //BONUS: prints all the palindrome strings inputted
    printf("Your palindrome words are: \n");
    for(i = 0; i < array_size; i++) {
        printf("%d.)%s \n", i+1, palindrome_array[i]);
    }

    return 0;
}


int isPalindrome(char check[]) {        // from string to individual characters
    int middle = strlen(check) / 2;     //gets the half of the string's length
    int length = strlen(check);

    for(int i = 0; i < middle; i++) {
        if(check[i] != check[length - i - 1]) {
            return 1;
        } 
    }
    return 0;
}   

Answer 1

str_array[i] = (char*) malloc(sizeof(char) * strlen(str_length));

You need one char more to accommodate the null terminating character.

        str_array[i] = malloc(sizeof(**str_array) * strlen(str_length) + 1);
        strcpy(str_array[i], str_length); 

You repeat this error in other places as well. You need to change all wrong allocations.

Some remarks:

1. Do not cast result of malloc. If the code does not compile you use C++ compiler to compiler C code which is not good.
2. Use objects instead of types in sizeof

Answer 2

Two problems. There are two occurrences of the
first problem. They are both the same, and are simply that you are not providing enough room for NULL terminator in your string allocations... The error is in this clause:

str_array[i] = (char*) malloc(sizeof(char) * strlen(str_length));
                                             ^^^^^^^^^^^^^^^^^^
    strcpy(str_array[i],str_length); 

Change it to :

 str_array[i] = malloc(strlen(str_length)+1);
                                         ^^
    strcpy(str_array[i],str_length);   

...to accommodate need for NULL character.

The same is true for the palindrome test:

palindrome_array[i] = malloc( strlen(str_length)+1);
                                                ^^
        strcpy(palindrome_array[i], str_length); 

Note - applies to both corrections above:

• Casting return of malloc() is not necessary and generally not a good idea in C.
• And sizeof(char) is always 1 by definition.

second problem occurs when at the end of the inputting of words, and the program attempts to output all palindromes, the array palindrome_array[i] was found to have one less element of array than needed. I will leave troubleshooting that problem to you, but Should you choose to use realloc()at some point, here is a discussion on how to do it correctly.

Answer 3

There are two main drawbacks.

To allocate a memory for a string you need to reserve space for the terminating zero character '\0' of the string.

So you need to write for example

str_array[i] = (char*) malloc(sizeof(char) * ( strlen(str_length) + 1 ) );

Also the array palindrome_array was not initialized.

char *str_array[array_size], *palindrome_array[array_size], str_length[100];

As not all tested strings are palindromes then some elements of the array still stay uninitialized for some values of the index i due to this if statement

    if(isPalindrome(str_array[i]) == 0) {
        palindrome_array[i] = (char*) malloc(sizeof(char) * strlen(str_length));
        strcpy(palindrome_array[i], str_length); 
        printf("'%s' is a palindrome \n", palindrome_array[i]);
    } else printf("'%s' is NOT a palindrome \n", str_array[i]);

As a result this for loop

for(i = 0; i < array_size; i++) {
    printf("%d.)%s \n", i+1, palindrome_array[i]);
}

can invoke undefined behavior.

You need to declare a separate index for the array and to use it instead of the index i.

In fact the array str_array is just redundant. It is enough to use these two arrays.

char *palindrome_array[array_size], str_length[100];

The parameter of the function isPalindrome should be declared with the qualifier const

int isPalindrome( const char check[]);

because the passed string is not changed within the function. Also it is much better when the function returns a npn-zero integer when a string is a palindrome and zero otherwise.

In the call of scanf you should specify the maximum length of the input string and the conversion specifier c should be removed. For example

scanf( " %99[^\n]", str_length );

Also it is unclear why there is used the magic number 5 in your program.

I would write the program the following way

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

int isPalindrome( const char s[] )
{
    size_t i = 0, n = strlen( s );

    while (i < n / 2 && s[i] == s[n - i - 1]) ++i;

    return i == n / 2;
}

int main( void )
{
    char **p = NULL;
    size_t total = 0;
    size_t count = 0;

    while (1)
    {
        char s[100];

        printf( "Enter a word (to exit just press Enter): " );

        if (scanf( "%99[^\n]%*c", s ) != 1 || s[0] == '\0') break;

        ++total;

        if (isPalindrome( s ))
        {
            printf( "\"%s\" is a palindrome\n", s );
            char **tmp = realloc( p, ( count + 1 ) * sizeof( char * ) );

            if (tmp == NULL)
            {
                puts( "Memory allocation error. It is impossible to store the string." );
                break;
            }

            p = tmp;

            p[count] = malloc( strlen( s ) + 1 );

            if (p[count] == NULL )
            {
                puts( "Memory allocation error. It is impossible to store the string." );
                break;
            }

            strcpy( p[count], s );
            ++count;
        }
        else
        {
            printf( "\"%s\" is NOT a palindrome\n", s );
        }
    }

    printf( "\nAmong the entered %zu words there are %zu palindrome(s).\n", total, count );
    if (count != 0)
    {
        puts( "They are:" );
        for (size_t i = 0; i < count; i++)
        {
            printf( "\"%s\"\n", p[i] );
        }
    }

    for (size_t i = 0; i < count; i++)
    {
        free( p[i] );
    }
    free( p );
}

Its output might look like

Enter a word (to exit just press Enter): 1
"1" is a palindrome
Enter a word (to exit just press Enter): 12
"12" is NOT a palindrome
Enter a word (to exit just press Enter): 122
"122" is NOT a palindrome
Enter a word (to exit just press Enter): 1221
"1221" is a palindrome
Enter a word (to exit just press Enter): 12321
"12321" is a palindrome
Enter a word (to exit just press Enter):

Among the entered 5 words there are 3 palindrome(s).
They are:
"1"
"1221"
"12321"