Finding palindromes and reverse strings in C

Question

I've created a program that will find if a word is a palindrome or a reverse string (emordnilap).

How it works:

The palindrome part works by reversing the string, and comparing the two strings using strcmp to see if they match one another. If they do match, it will return true, if they do not match false.

How it checks if the word is a reverse string is by running through a wordlist line by line, stripping the string using a strip function and testing if the word is in the wordlist. Examples of usage:

e@ubuntu:~/bin/c$ ./epi.exe -h
usage: epi -h -t -s [string] [file]

-h       Print this help and exit

-t       Run the test strings.

-s       Input a string to check if it is either a palindrome or
         a emordinlap, followed by the path to a file.
e@ubuntu:~/bin/c$ ./epi.exe -t

'a' is a palindrome, it is the same forward and backwards
Reverse of 'a' is a emordnilap, it spells a word backwards.


'ab' is not a palindrome, it is not the same forward and backwards
Reverse of 'ab' is not a emordnilap, it does not spell a word backwards


'abc' is not a palindrome, it is not the same forward and backwards
Reverse of 'abc' is not a emordnilap, it does not spell a word backwards


'another' is not a palindrome, it is not the same forward and backwards
Reverse of 'another' is not a emordnilap, it does not spell a word backwards


'cbc' is a palindrome, it is the same forward and backwards
Reverse of 'cbc' is not a emordnilap, it does not spell a word backwards


'|0|' is a palindrome, it is the same forward and backwards
Reverse of '|0|' is not a emordnilap, it does not spell a word backwards


'palindrome' is not a palindrome, it is not the same forward and backwards
Reverse of 'palindrome' is not a emordnilap, it does not spell a word backwards // technically emordnilap is not a word

e@ubuntu:~/bin/c$ ./epi.exe -s 

You must provide a string to test.

usage: epi -h -t -s [string] [file]

-h       Print this help and exit

-t       Run the test strings.

-s       Input a string to check if it is either a palindrome or
         a emordnilap, followed by the path to a file.
e@ubuntu:~/bin/c$ ./epi.exe -s test

You must pass a valid file path to use as a wordlist.

usage: epi -h -t -s [string] [file]

-h       Print this help and exit

-t       Run the test strings.

-s       Input a string to check if it is either a palindrome or
         a emordinlap, followed by the path to a file.
e@ubuntu:~/bin/c$ ./epi.exe -s test test.txt

'test' is not a palindrome, it is not the same forward and backwards
Reverse of 'test' is not a emordnilap, it does not spell a word backwards

e@ubuntu:~/bin/c$ ./epi.exe -s tset test.txt

'tset' is not a palindrome, it is not the same forward and backwards
Reverse of 'tset' is a emordnilap, it spells a word backwards.

e@ubuntu:~/bin/c$ 

Key points that I would like to focus on, of course critique the entire program:

• Is there a better way to reverse a string?
• Running through a file list, am I doing it properly?
• Could this program be simplified?

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

static void *helpPage(void)
{
    puts("usage: epi -h -t -s [string] [file]\n");
    puts("-h       Print this help and exit\n");
    puts("-t       Run the test strings.\n");
    puts("-s       Input a string to check if it is either a palindrome or");
    puts("         a emordnilap, followed by the path to a file.");;
}

static char *reverse(char *str)
{
    // reverse a given string

    char tmp, *src, *dst;
    size_t len;

    if (str != NULL)
    {
        len = strlen(str);
        if (len > 1)
        {
            src = str;
            dst = src + len - 1;

            while (src < dst)
            {
                tmp = *src;
                *src++ = *dst;
                *dst-- = tmp;
            }
        }
    }
    return str;
}

static char *strip(char *s)
{
    // strip a string of a new line

    return strtok(s, "\n");
}

static bool checkEpi(char *reversed, char *filePath)
{
    // check if the word matches in a given file
    // or if the word is an emordnilap

    FILE *wordList;
    char *line = NULL;
    size_t len = 0;
    ssize_t read;

    wordList = fopen(filePath, "r");
    if (wordList == NULL)
    {
        perror("Failed to open file :"); // file probably doesn't exit
    }
    while ((read = getline(&line, &len, wordList)) != -1) // read the file line by line
    {
        if (strcmp(strip(line), reversed) == 0)
        {
            return true; // return true if the word matches
        }
    }
    fclose(wordList);
}

static bool checkPali(char *origin, char *reversed)
{
    // check if a given word is a palindrome or not

    if (strcmp(origin, reversed) == 0)
    {
        return true;
    }
    else
    {
        return false;
    }
}

static void checkAll(char *origin, char* reverse, char *filePath)
{
    // basically a main function to check if it's a palindrome or a emordnilap

    bool paliRes = checkPali(origin, reverse);
    bool epiRes = checkEpi(reverse, filePath);
    if (paliRes == true)
    {
        printf("\n'%s' is a palindrome, it is the same forward and backwards\n", origin);
    }
    else
    {
        printf("\n'%s' is not a palindrome, it is not the same forward and backwards\n", origin);
    }

    if (epiRes == true)
    {
        printf("Reverse of '%s' is a emordnilap, it spells a word backwards.\n\n", origin);
    }
    else
    {
        printf("Reverse of '%s' is not a emordnilap, it does not spell a word backwards\n\n", origin);
    }
}

int main(int argc, char *argv[])
{
    if (argv[1] == NULL)
    {
        puts("\nYou failed to pass a valid flag...\n");
        helpPage();
        return 1;
    }
    else
    {

        char *testStrings[] = {"a", "ab", "abc", "another", "cbc", "|0|", "palindrome"};
        int i;
        char s[10000];
        char *defaultWordList = "/usr/share/dict/american-english";
        size_t optInt;

        for (optInt = 1; optInt < argc && argv[optInt][0] == '-'; optInt++)
        {
            switch(argv[optInt][1])
            {
                case 't':
                {
                    for (i = 0; i < sizeof(testStrings) / sizeof(testStrings[0]); i++)
                    {
                        strcpy(s, testStrings[i]);
                        char *origin = testStrings[i];
                        char *revStr = reverse(s);
                        checkAll(origin, revStr, defaultWordList);
                    }
                    return 0;
                }

                case 's':
                {
                    if (argv[2] == NULL)
                    {
                        puts("\nYou must provide a string to test.\n");
                        helpPage();
                        return 1;
                    }
                    else if (argv[3] == NULL)
                    {
                        puts("\nYou must pass a valid file path to use as a wordlist.\n");
                        helpPage();
                        return 1;
                    }
                    else
                    {
                        strcpy(s, argv[2]);
                        char *revStr = reverse(s);
                        checkAll(argv[2], revStr, argv[3]);
                        return 0;
                    }
                }

                case 'h': helpPage(); return 0;
            }
        }
        return 0;
    }
}

Answer 1

First off: I really like reading your code. The code style looks clean and the intend of all variables seem clear to the reader - good naming. This was pretty quick and I may revisit this post later - don't have enough time right now.

Here are some things I noticed (in no particular order):

• You declared the function helpPage(void) to return void *, but never return anything. You probably just meant to write void. Also I would've replaced helpPage with printHelpPage.

• Declare variables when you first use them. This makes the intend more clear to the reader. Example in reverse(char *str):

  // Reverse a given null-terminated string
  static char *reverse(char *str)
  {
      if (str != NULL)
      {
          size_t len = strlen(str);
  
          if (len > 1)
          {
              char *src, *dst;
              src = str;
              dst = src + len - 1;
  
              while (src < dst)
              {
                  char tmp = *src;
                  *src++ = *dst;
                  *dst-- = tmp;
              }
          }
      }
      return str;
  }
  

  I think this makes the use of each variable easier to understand, otherwise the reader could be confused (What do I need all these variables for?).

• I'm not sure if creating an extra function strip(char *s) for a single strtok call is really necessary.

• Pay attention to const correctness in checkEpi: Mark reversed and filePath const, since checkEpi doesn't modify them through these pointers:

  static bool checkEpi(const char *reversed, const char *filePath);
  

  Same with checkPali / checkAll.

• Handle failed fopen. You do print and error message using perror, but the function should return right after printing the error.
• Memory leak in checkEpi. getline will allocate memory for line and you are responsible for freeing it later on.
• I don't see a return statement in checkEpi beside from the return true. Explicitly return false!
• checkPali shouldn't expect the original and the reversed string. The function should check if a single string is a palindrome (the reversing happens inside checkPali) - same thing in checkAll.

• Take a look at your current checkPali implementation. Never write code like this:

  if (something) {
      return true;
  } else {
      return false;
  }
  

  but instead write return something; right away .

• checkAll: Your current code assigns the return values of checkPali and checkEpi to variables named and paliRes and epiRes and later on checks them with paliRes == true. Consider renaming these variables to isPalindrome and isEmordnilap and using them like if(isPalindrome) ..., etc.
• Minor: I probably would've defined testStrings and defaultWordList at the top of the file via the #define-directive.
• Minor: You accidentally put one semicolon too many into helpPage(void).

This is what I came up with:

(The code should just give you a rough example on how to do it. I'm currently in a rush, so please excuse any mistakes made)

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

static void printHelpPage(void) {
    puts("Usage: epi -h -t -s [string] [file]\n");
    puts("-h       Print this help and exit\n");
    puts("-t       Run the test strings.\n");
    puts("-s       Input a string to check if it is either a palindrome or");
    puts("         a emordnilap, followed by the path to a file.");
}

// In-place reverse null-terminated string
static char *reverse(char *str) {
    size_t len;
    if (str != NULL && (len = strlen(str)) > 1) {
        char *src, *dest;
        src = str;
        dest = src + len - 1;

        while (src < dest) {
            char tmp = *src;
            *src++ = *dest;
            *dest-- = tmp;
        }
    }
    return str;
}

// Return values: -1 on error, 0 for no Emordnilap, 1 for Emordnilap
static int checkEmordnilap(const char *str, const char *filePath) {
    char *reversed = malloc(strlen(str) + 1);

    if (!reversed) {
        perror("Failed to allocate memory to store reversed string ");
        return -1;
    }

    strcpy(reversed, str);
    reverse(reversed);

    FILE *wordListFile = fopen(filePath, "r");

    if (!wordListFile) {
        free(reversed);
        perror("Failed to open wordList ");
        return -1;
    }

    char *line = NULL;
    size_t linecap = 0;
    ssize_t linelen;

    while((linelen = getline(&line, &linecap, wordListFile)) > 0) {
      if (strcmp(strtok(line, "\n"), reversed) == 0) {
        return 1;
      }
    }

    fclose(wordListFile);
    free(reversed);

    return 0;
}


// Return -1 on error, 0 on no palindrome, 1 on palindrome
static int checkPalidrome(const char *str) {
    char *reversed = malloc(strlen(str) + 1);

    if (!reversed) {
        perror("Failed to allocate memory to store reversed string.\n");
        return -1;
    }

    strcpy(reversed, str);
    reverse(reversed);

    int result = strcmp(str, reversed) == 0;
    free(reversed);

    return result;
}

static void printStatus(const char *origin, const char *filePath) {
    int isPalindrome, isEmordnilap;
    isPalindrome = checkPalidrome(origin);
    isEmordnilap = checkEmordnilap(origin, filePath);

    if (isPalindrome == -1 || isEmordnilap == -1) {
        // We already see the error message printed from checkPalindrome/checkEmordnilap.
        return;
    }

    if (isPalindrome) {
        printf("\n'%s' is a palindrome, it is the same forward and backwards\n", origin);
    } else {
        printf("\n'%s' is not a palindrome, it is not the same forward and backwards\n", origin);
    }

    if (isEmordnilap) {
        printf("Reverse of '%s' is a emordnilap, it spells a word backwards.\n\n", origin); 
    } else {
        printf("Reverse of '%s' is not a emordnilap, it does not spell a word backwards\n\n", origin);
    }
}

#define TEST_STRINGS { "a", "ab", "abc", "another", "|0|", "palindrome" }
#define DEFAULT_WORD_LIST "/usr/share/dict/propernames"

int main(int argc, char *argv[]) {
  if (argc < 2) {
      printHelpPage();
      return 1;
  }

  for (int argNum = 1; argNum < argc && argv[argNum][0] == '-'; ++argNum) {
      switch(argv[argNum][1]) {
          case 't': {
              char *testStrings[] = TEST_STRINGS;
              for (size_t wordIndex = 0; wordIndex < sizeof(testStrings)/sizeof(testStrings[0]); ++wordIndex) {
                  printStatus(testStrings[wordIndex], DEFAULT_WORD_LIST);
              }
              break;
          }
          case 's':
              if (argc < 3) {
                  puts("\nYou must provide a string to test.\n");
                  printHelpPage();
                  return 1;
              }
              char *wordList = argc < 4 ? DEFAULT_WORD_LIST : argv[4];
              printStatus(argv[2], wordList);
              break;
          case 'h':
              continue;
          default:
              printHelpPage();
              break;
      }
  }

  return 0;
}

Answer 2

The (str != NULL) is not needed as str, as a string, is never NULL. In C, a string is a sequence (e.g. array) of characters up to and including the terminating null character.

Still it is not a bad idea to check against NULL for as a pointer, char * sometimes will be assigned NULL.

The if (len > 1) can be eliminated by taking advantage of starting with right assigned to the address of the null character.

static char *reverse(char *str) {
  char *left = str;
  char *right = str + strlen(str);
  while (right > left) {
    right--;
    char tmp = *right;
    *right = *left;
    *left = tmp;
    left++;
  }
  return str;
}