-6

It is a simple program to check whether string is palindrome or not. I made following code in the program.

program to check string is palindrome or not

When i compile it, there is no error but when i try to run the .exe file, i always get following message.

.exe is not responding

phuclv
  • 37,963
  • 15
  • 156
  • 475
Nitish
  • 1
  • 1
  • 4
    [do not put code and text output as images](https://meta.stackoverflow.com/q/303812/995714). copy and paste them here – phuclv Oct 28 '17 at 14:07
  • 1
    Undefined behavior if `strlen(str)` is 0.... Also, [never use `gets()` under any circumstances](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used). This dangerous function is no longer even part of the C language. – ad absurdum Oct 28 '17 at 14:12

1 Answers1

0

The fundamental problem here is that the loop exit condition can never be met, and this leads to an infinite loop:

(i != j || i != j - 1)

This condition is logically equivalent to:

!(i == j && i == j - 1)

which is obviously always true, so the loop continues indefinitely. The loop only needs to continue so long as j > i.

There is another problem here; namely that the dangerous function gets() should never be used. This function was deprecated in C99 and completely removed from the language in C11. One alternative is to use fgets() instead. Note that this function keeps the newline (if there is room in the buffer), so you will need to remove this after getting the input. Also, characters may be left in the input stream if the buffer is too small. For this reason, it would be better to declare a generously sized input buffer to reduce the risk of problems here. There is no reason not to use an input buffer holding 1000 characters, and I usually just use 4096 for something like this. Memory is cheap.

Further, there is a risk of undefined behavior in the posted code, since the input string may be empty. In this case, strlen(str) would be 0, so in the first execution of the loop body j would be decremented to -1. But the array access str[-1] is out of bounds, and leads to undefined behavior.

This problem can be fixed by checking that j is positive before the first decrement. Note that size_t is the correct type for array indices, as it is an unsigned integer type that is guaranteed to be able to hold any array index. Also note that the strlen() function returns a value of type size_t, not int.

Here is a modified version of the posted code. The size_t type is used for array indices. The length of the input string is stored in j, which is then decremented only if it is a positive value; this sets j to the index of the character preceding the null terminator so long as the input string is not an empty string. The loop continues while j is greater than i and the characters indexed by these values match. After the loop terminates, str[i] and str[j] should agree; if they do not, then the input was not a palindrome.

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

#define BUF_SZ  4096

int main(void)
{
    char str[BUF_SZ];

    printf("Enter string:\n");
    fgets(str, sizeof str, stdin);           // Never use gets()
    str[strcspn(str, "\r\n")] = '\0';        // remove '\n'

    size_t i = 0;
    size_t j = strlen(str);
    if (j > 0) {
        --j;
    }

    while (i < j && str[i] == str[j]) {
        ++i;
        --j;
    }

    if (str[i] == str[j]) {
        puts("string is palindrome!!");
    } else {
        puts("string is not palindrome!!");
    }

    return 0;
}
ad absurdum
  • 19,498
  • 5
  • 37
  • 60