Echo All Palindromes, in C

Question

I love the ideas presented in Brian Kernighan and Rob Pike's book, "The UNIX Programming Environment," where they focus on the point of working within an environment where you can put together many (small, precise, well understood) programs on the command line to accomplish many programming tasks.

I'm brushing up on strict ANSI C conventions and trying to stick to this philosophy. Somewhere in this book (I can get an exact page number if needed) they suggest that all programs in this environment should adhere to the following principles:

1. If input is presented on the command line, as an argument to the program itself, process that input.

2. If no input is presented on the command line, process input from stdin.

Here's a C program I wrote that will echo any input (numeric or alphabetic) that is a palindrome. My question specifically:

Is this a well behaved C program? In other words, is this what Kernighan and Pike were suggesting is the optimal behavior for a command line application like this?

#include <stdio.h>
#include <string.h> /* for strlen */

int main(int argc, char* argv[]) {

    char r_string[100];

    if (argc > 1) {

        int length = (int)strlen(argv[1]);

        int i = 0;
        int j = length;
        r_string[j] = (char)NULL;
        j--;
        for (i = 0; i < length; i++, j--) {
           r_string[j] = argv[1][i];
        }

        if (strcmp(argv[1], r_string) == 0) {
            printf("%s\n", argv[1]);
        }

    } else {

        char* i_string;

        while (scanf("%s", i_string) != EOF) {

            int length = (int)strlen(i_string);

            int i = 0;
            int j = length;
            r_string[j] = (char)NULL;
            j--;
            for (i = 0; i < length; i++, j--) {

               r_string[j] = i_string[i];
            }

            if (strcmp(i_string, r_string) == 0) {

                printf("%s\n", i_string); 
            }
        }
    }

    return 0;
}

Answer 1

Yes, I think that you are following the R&K advice. As Hugo said, you could take the argumentas a filename, bu,t IMHO, for this simple program, I'd say that taking the parameter as the palindrome itself may make more sense.

Also, if you allow me extra advice, I would separate the functionality of reading a string from checking whether it is a palindrome or not, because you have that code duplicated right now.

int ispalindrome(const char* c) { 
   size_t len = strlen(c);
   size_t limit = len/2;
   size_t i;
   for (i = 0; i < limit; i++) {
     if(c[i]!=c[len-i-1]) break; /* Different character found */
   }
   return i==limit; /* If we reached limit, it's a palyndrome */
}

Of course, I am pretty sure this can be improved (it may even have a bug, I am typping quite fast), but once that you have your string, be either from command line or user input, you can call this function or a functiom like this.

NOTE: Edited to reflect comment from Mark, thanks a lot, Mark!

Answer 2

One problem that you have is a potential buffer overflow because you are writing an input of arbitrary length into a buffer with a fixed size. You can fix this by rejecting too long inputs or creating an array of the correct size dynamically. I would avoid using scanf.

Regarding the actual algorithm, you don't need to copy the string reversed and then compare the two strings. You could do the check using only a single copy of the string and a pointer at both ends, both moving in towards the middle.

Here is some code to show the principle:

char* a = /* pointer to first character in string */;
char* b = /* pointer to last character in string (excluding the null terminator) */;

while (a < b && *a == *b)
{
    a++;
    b--;
}

if (a >= b)
{
    // Is palindrome.
}

I agree with Javier that you factor the palindrome checking code out into a separate function.