Getting a “segmentation Fault” on Caesar, Pset2

Question

This is my code for Ceasar (pset2) of cs50.

I am able to compile my program.

However, on trying to execute it, I get a seg fault. Also, on using the debugger, I don't get a seg fault, but a ^D before the ciphertext is displayed. Much like this:

plaintext: HELLO ciphertext: ^DIFMMP

Could you point out to me where the problem lies?

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

bool is_numerical(string e);

int main(int argc, string argv [])
{
  if (argc == 2 && is_numerical(argv[1] == true))
   {
      string t = argv [1];
      int k = atoi ( t );
      string s = get_string ("plaintext: ");
      printf ("ciphertext:" );
      for (int i = 0, n = strlen(s); i < n; i++)
      {
         char c = s[i];
         if (isalpha (c))
         {
            if (isupper(c))
            {
               int x = ((int) c - 65 + k) % 26 + 65;
               printf ("%c", (char) x);
            }
            else
            {
               int x = ((int) c - 97 + k) % 26 + 97;
               printf ("%c", (char) x);
            }
         }
         else
         {
            printf ("%c", c);
         }
      }

      printf ("\n");
      return 0;
   }
   else
   {
       printf("Usage: ./caesar key \n");
       return 1;
   };
}

bool is_numerical(string e)
{
   for (int i = 0, n = strlen(e); i < n; i++)
   {
      if (!isalnum (e))
         return false;
   }
   return true;
}

Thank you.

Answer 1

There seem to be quite a lot of things wrong with this.

First, let's acknowledge the elephant in the room-

if (argc == 2 && is_numerical(argv[1] == true))

This checks if argc is equal to 2 and if is_numerical returns true when the argument is argv[1] == true, argv[1] will be true when argc is equal to 2. So really, you're passing an integer value to is_numerical everytime, a value of 1 - but it expects a value of char* or string.

You probably meant to do is_numerical(argv[1]) == true. i.e, pass argv[1] to is_numerical and compare the return value to true. You can also completely omit the true part since that's redundant in a boolean expression.

if (argc == 2 && is_numerical(argv[1]))

Now, you have a fatal mistake in your is_numerical function.

if (!isalnum(e))

isalnum takes a value of type char (actually int, but char will get promoted anyway). You're passing e to it. Guess what type is e, it's string, or char*. Shouldn't you be passing each character of the string, so e[i] inside that loop?

if (!isalnum(e[i]))

There may be more algorithmic issues in your code that aren't immediately apparent. But the fatal mistake at is_numerical is the reason behind the segmentation fault.

Word of advice, always compile with -Wall to catch these mistakes during compilation.