Vigenere Segmentation understanding

Question

I'm currently working on 'Vigenere' on the CS50 online course and i came across this issue that i totally believe comes down to problem in my understanding.

please find my code below...

// INCLUDE THE APPROPRIATE LIBRARIES

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

int main (int argc, string argv[])

// OBTAIN A VALID INPUT FOR THE PROGRAMME

{

    int mki = 2;

    if (argc < mki || isalpha(argv[1]) == false)

    {

        printf ("This programme requires an alhabetical key input in order 
        to execute. (e.g. vigenere abc)\n");

    }

}

Now my thinking when writing this was "I want to check the users input is as efficiently as possible, checking the that the user actually puts in a character after the program name, and check that the character is alphabetical."

The code compiles, and runs correctly when I fail to enter a character after the program name e.g (./vigenere) see below.

~/workspace/workspace/week2/ $ ./vigenere
This programme requires an alhabetical key input in order to execute. (e.g. 
vigenere abc)

But i get a "segmentation fault" when I run the program with any character after the programme name. (e.g. /vigenere x)

~/workspace/workspace/week2/ $ ./vigenere 1
Segmentation fault
~/workspace/workspace/week2/ $ ./vigenere a
Segmentation fault

My (very basic) understanding of segmentation fault is that the computer has been asked for something that essentially does not exist within the program. However i thought i had provided an accurate pointer for the computer with "isalpha(argv[1]) == false"

basically ... how have I messed this up? and where is the gap in my understanding?

Thank you all in advance

Pot Noodle.

additional note:

having now applied the logic that "isaplha" must be applied in a loop i wrote the following code:

// INCLUDE THE APPROPRIATE LIBRARIES

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

int main (int argc, string argv[])

// OBTAIN A VALID INPUT FOR THE PROGRAMME

// CHECK THAT THE USER INPUTS AT LEAST ONE ARGUMENT ALONGSIDE VIGENERE

{

int mki = 2;

if (argc < mki)

{

printf ("This programme requires an alhabetical key input in order to execute. (e.g. vigenere abc)\n");

return 1;

}

// CHECK THAT THE ARGUMENT GIVEN IS COMPOSED OF ALPHABETICAL CHARACTERS

int av1c = strlen(argv[1]);

int av1it = 0;

for (; av1it < av1c; av1it++)

{

    if (!isalpha(argv[av1it]))

    {

        printf ("This programme requires a completely alphabetical key input in order to execute. (e.g. vigenere abc)\n");

        return 1;
    }

}

}

Running the above code gives me the same result as the first set of code.

~/workspace/workspace/week2/ $ ./vigenere a
Segmentation fault

....so yeah, i'm clearly missing something here. can anybody help?

thanks,

Pot Noodle

Answer 1

This is a very, very common error for new programmers. The problem is the actual call to isalpha(). The isalpha() function takes a single char as input, but argv[1] is a string, not a char. Simply put, the code is trying to stuff a string down isalpha's throat and it chokes, resulting in a seg fault.

You need to check each char in the string. Hint: a for loop is your friend.

As a side note,Keep this in mind. isalpha() and all its cousins return integers, not bool. While they always return 0 when false, the value returned for true will be a power of 2, depending on which function it is. Since a non-zero is always treated as true, it is best practice to use one of the following instead of equating/comparing to true or false:

     if( isalpha(x) )  ...
     if( !isalpha(x) ) ...

These are also more efficient because it eliminates a calculation step. ;-)

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)