my C function is modifying a char array it shouldn't because it's deliberately out of scope

Question

I wrote a little program in C that checks if a word of phrase is a palindrome. One function does the checking after calling another function to remove the spaces in a phrase. The app first asks the user to enter a word or phrase. Then it strips out the spaces in a separate function then evaluates the function to see if its a palindrome and return a bool... The printf statements use the original string (or should) but the resulting printf uses the modified string instead. I don't see where is gets it. Here is the code:

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

bool isPalindrome(char string[]);
char* stripSpaces(char *noSpace);

int main(void)
{
    char usrStr[250];


    printf("Please enter a string and I will check if it is a palindrome...");
    scanf( "%250[^\n]", usrStr);
    
    printf("\n");
    isPalindrome(usrStr) ? printf("\"%s\" is a palindrome", usrStr) : printf("\"%s\" is not a palindrome", usrStr);

    printf("\n");

    return 0;
}

bool isPalindrome(char* string)
{
    char* newStr = stripSpaces(string);
    
    int middle = strlen(newStr) / 2;
    int len = strlen(newStr);

    for(int x = 0; x < middle; x++)
        if(newStr[x] != newStr[len - x - 1])
            return false;  
    // Check first char againse last char - then 2 char against 2nd last char and so on until mismatch found.
    // If no mismatch then the string is a palindrome
    
    return true;
}

char* stripSpaces(char *noSpace)
{
    int length = strlen(noSpace);
    int count = 0;
    for(int idx = 0; idx < length; idx++)
    {
        if(noSpace[idx] != ' ')
        {
            noSpace[count] = noSpace[idx];
            count++;
        }
    }
    noSpace[count] = '\0';

    return noSpace;

}

Any thoughts? Cheers Charles

I tried making usrStr a const to no avail. I expected usrStr to not get modified as it is out of scope in the stripSpaces function.

Answer 1

noSpace is a pointer. If you change a pointer it will change out of scope too.

for a new string you can use malloc

char* stripSpaces(char *noSpace)
{
    int length = strlen(noSpace);
    char *ret = malloc( length );
    int count = 0;
    for(int idx = 0; idx < length; idx++)
    {
        if(noSpace[idx] != ' ')
        {
            ret[count++] = noSpace[idx];
        }
    }
    ret[count] = '\0';

    return ret;

}

Answer 2

When you pass an array to a function, it decays to a pointer to its first element. While you've made a copy of the pointer the data it points to is the same.

If you wish to not modify the original string, you need to copy its contents and work on the copy rather than the original. The strdup function makes this very simple.

char* stripSpaces(char *noSpace)
{
    int length = strlen(noSpace);
    char * str_copy = strdup(noSpace);
    int count = 0;
    for(int idx = 0; idx < length; idx++)
    {
        if(str_copy[idx] != ' ')
        {
            str_copy[count] = str_copy[idx];
            count++;
        }
    }
    str_copy[count] = '\0';

    return str_copy;
}

Keep in mind, though, that this is dynamically allocating memory, so you have to de-allocate when you're done.

bool isPalindrome(char* string)
{
    char* newStr = stripSpaces(string);
    
    int middle = strlen(newStr) / 2;
    int len = strlen(newStr);

    for(int x = 0; x < middle; x++)
        if(newStr[x] != newStr[len - x - 1]) {
            free(newStr);
            return false; 
        }
    
    free(newStr);
    return true;
}

You should also check that strdup succeeded in allocating memory. If it fails, it will return a null pointer.

Answer 3

Look for example at your own declarations of the function isPalindrome

bool isPalindrome(char string[]);

and

bool isPalindrome(char* string)
//...

These function declarations are equivalent. The complier adjusts a parameter having an array type to a parameter having a pointer type to the element type of the array.

On the other hand, an array used as an argument expression is implicitly converted to a pointer to its first element.

Within the function isPalindrome

bool isPalindrome(char* string)
{
    char* newStr = stripSpaces(string);
    //...

you are explicitly passing to the function stripSpaces the pointer to the first element of the original array usrStr. So the function stripSpaces using this pointer and the pointer arithmetic in the expression noSpace[count] that is equivalent to the expression *( noSpace + count ) can change any element of the original array.

In C passing an object indirectly through a pointer to it to a function means passing the object by reference because using the pointer you can change the original object by means of dereferencing the pointer.

As for other characteristics of your code then for example instead of this used format string

scanf( "%250[^\n]", usrStr);

you have to write

scanf( "%249[^\n]", usrStr);

Otherwise if the user indeed will enter 250 characters then the function scanf will write the terminating zero character '\0' outside the array that results in undefined behavior.

The function strlen has the unsigned integer return type size_t. So you have to write for example

size_t len = strlen(newStr);

instead of

int len = strlen(newStr);

The function isPalindrome shall not change a passed to it string. So its single parameter should be declared with qualifier const like

bool isPalindrome( const char* string);

The approach when an auxiliary array is created to store a string without blanks is inefficient and unsafe. The function shall not require an additional memory to check whether a string is a palindrome.

Also pay attention to that you should process also tab characters '\t' that also can separate words in a string.

I would declare and define the function the following way as shown in the demonstration program below.

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

bool isPalindrome( const char *s )
{
    const char *left = s, *right = s + strlen( s );

    while (true)
    {
        while (isblank( ( unsigned char )*left )) ++left;
        while (right != left && isblank( ( unsigned char )*--right ));

        if (left == right || *left != *right) break;

        ++left;
    }

    return left == right;
}

int main( void )
{
    const char *s = " 1 ";
    printf( "\"%s\" is %spalindrome.\n", s, isPalindrome( s ) ? "" : "not " );

    s = " 1 1 ";
    printf( "\"%s\" is %spalindrome.\n", s, isPalindrome( s ) ? "" : "not " );

    s = " 1 2 1 ";
    printf( "\"%s\" is %spalindrome.\n", s, isPalindrome( s ) ? "" : "not " );

    s = " 1 1 2 ";
    printf( "\"%s\" is %spalindrome.\n", s, isPalindrome( s ) ? "" : "not " );
}

The program output is

" 1 " is palindrome.
" 1 1 " is palindrome.
" 1 2 1 " is palindrome.
" 1 1 2 " is not palindrome.