Vigenere code has Segmentation error

Question

I've been working on the vigenere.c problem from pset2 and am struggling to find the error in my code currently because whenever I try to run it my code crashes. Clearly there is something wrong but I don't exactly know where to start. Thanks for help.

int main(int argc, string argv[])
{
  if(argc == 2)//checks if key was given
  { 
    if(isalpha(argv[1]))//checks if key is letters
    {
        int cypher_number, arglet, keylen;
        char cypher_letter; 
        string secret_message, key;

        keylen = strlen(argv[1]);//string length of key
        key = argv[1];//key
        secret_message = GetString();
        int messlen = strlen(secret_message);

        for(int letter = 0; letter < messlen; letter++)
        {
            if(islower(secret_message[letter]))
            {
                for(int i = 0; i <= keylen; i++)
                {
                    if(isupper(key[i]))
                    {
                        arglet = key[i] - 65;//subtracts A
                        cypher_number = ((secret_message[letter] + arglet) - 65)  % 26;
                        cypher_letter = cypher_number + 65;
                        printf("%c", cypher_letter);
                        i = i % keylen;

                    }
                    if(islower(key[i]))
                    {
                        arglet = key[i] - 97;//subtracts a
                        cypher_number = ((secret_message[letter] + arglet) - 97)  % 26;
                        cypher_letter = cypher_number + 97;
                        printf("%c", cypher_letter);
                        i = i % keylen;
                    }
                }
            }
            if(isupper(secret_message[letter]))
            {
                for(int i = 0; i <= keylen; i++)
                {
                    if(isupper(key[i]))
                    {
                        arglet = key[i] - 65;//subtracts A
                        cypher_number = ((secret_message[letter] + arglet) - 65)  % 26;
                        cypher_letter = cypher_number + 65;
                        printf("%c", cypher_letter);
                        i = i % keylen;
                    }
                    if(islower(key[i]))
                    {
                        arglet = key[i] - 97;//subtracts a
                        cypher_number = ((secret_message[letter] + arglet) - 97)  % 26;
                        cypher_letter = cypher_number + 97;
                        printf("%c", cypher_letter);
                        i = i % keylen;
                    }
                }
            }
            else
            {
                printf("%c", secret_message[letter]);
            }
        printf("\n");
        }    
    }
    else
    {
        printf("NO!\n");
        return 1;
    }
}  
else
{
    printf("NO.\n");
    return 1;
}
}

Answer 1

Your segmentation fault is due to the following line:

if(isalpha(argv[1]))//checks if key is letters

isalpha() tests to see if a single character is an alpha. It does not operate on whole strings. When you try to stuff a string into it, it blows up. You need to run a loop across the string, checking each element in argv, i.e., argv[1][i].

As a side note, I noticed that your tests to validate argc and argv completely wrap around the program code. In other words, it looks like this:

if(argc == 2 )
{
    do the entire program
}
else
{
    handle the error.
}

This is a bad practice because it introduces too many opportunities for error while writing and later while maintaining code. Nesting checks within each other has the same problems.

Instead, you should simply check for the error condition at the start of the program and if it fails, add code to either correct the problem, ask for new input, or exit the program at that point. This compartmentalizes the code so that the testing is localized and doesn't encapsulate the entire program.

Structurally, it would look like this:

if(argc != 2)
{
    // print an error message if necessary
    return 1;   // the number can be whatever you need it to be.
               // no number or 0 is usually used for successful completion.
}

// rest of the program follows.

It's simple, it's compartmentalized into a small block and it's easy to understand.

You have other issues, but this will correct your seg fault. After you rework the code a little, if you have more problems, those would be a new question. ;-)

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

Answer 2

The key most probably won't be the same length as the text user inputs to encrypt. So using the same index in the loop for both key and the text user inputs is not a good idea. At some point the index will be greater than the length of the key and segmentation fault will occur.

Solutions:

keep a separate index for key and increment it with in the loop. Also after each iteration make sure u reset it to 0 if index of key becomes greater than length of key.