Having problem with vigenere in cs50. Keeps returning segmentation error, even though I solved all the errors compiler pointed out

Question

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

char argument_upper (int key, char plaintext);
char argument_lower (int key, char plaintext);

int main(int argc, string argv[])
{
    string argument = argv[1];// stores key
    bool command_alpha;// To check if command line argument is all alpha
    int arg_index = 0;// stores index value of key
    int key;//To store number of ascii value to move

    if (argc != 2)// checks arg count
    {
        printf("Invalid command line arguments\n");
        return 1;
    }
    for (int i = 0; i <= strlen(argv[1]); i++)// checks if arg has all alphabets
    {
        command_alpha = isalpha(argv[1]);
        if (command_alpha == false)
        {
            printf("Invalid command line arguments\n");
            printf("%s\n", argument );
            return 1;
        }
    }



    printf("plaintext: ");
    string plaintext = get_string();

    printf("ciphertext: ");
    for (int j = 0; j < strlen(plaintext); j++)// Goes over every plain text character
    {
        bool plain_alpha = isalpha(plaintext[j]);// Checks if plain text char is alpha
        bool plain_upper = isupper(plaintext[j]);// Checks if plain text char is upper
        bool plain_lower = islower(plaintext[j]);// Checks if plain text char is lower
        bool arg_upper = isupper(argument[arg_index]);// Checks if argument char is upper
        bool arg_lower = islower(argument[arg_index]);// Checks if argument char is lower
        key = argument[arg_index];


        if (plain_alpha == true)
        {
            if (plain_upper == true)
            {

                if (arg_upper == true)
                {
                    key -= 65;
                    printf("%c", argument_upper (key, plaintext[j]));
                    arg_index += 1;
                    if (arg_index == strlen(argument))
                    {
                        arg_index -= strlen(argument);
                    }
                }
                else if (arg_lower == true)
                {
                    key -= 97;
                    printf("%c", argument_lower (key, plaintext[j]));
                    arg_index+= 1;
                    if (arg_index == strlen(argument))
                    {
                        arg_index -= strlen(argument);
                    }

                }


            }

            else if (plain_lower == true)
            {
                if (arg_upper == true)
                {

                    printf("%c", argument_upper (key, plaintext[j]));

                    arg_index += 1;
                    if (arg_index == strlen(argument))
                    {
                        arg_index -= strlen(argument);
                    }
                }
                else if (arg_lower == true)
                {

                    printf("%c", argument_lower (key, plaintext[j]));

                    arg_index +=1;
                    if (arg_index == strlen(argument))
                    {
                        arg_index -= strlen(argument);
                    }
                }
            }
        }
        else
        {
            printf("%c", plaintext[j]);
        }
    }
    printf("\n");

}

char argument_upper (int key, char plaintext)
{
    key -= 65;
    int newposition = plaintext + key;

    //Keeps checking if new position is going higher the alpha value in ASCII
    while (newposition > 90)
    {
        newposition -= 91;  
        newposition += 65;
    }

    return newposition;

}

char argument_lower (int key, char plaintext)
{
    key -= 97;
    int newposition = plaintext + key;

    //Keeps checking if new position is going higher the alpha value in ASCII
    while (newposition > 122)
    {
        newposition -= 123;  
        newposition += 97;
    }

    return newposition;


}

Answer 1

I'd segfault on that as well, for several reasons.

1. You access argv[1] before checking argc (possible segfault on calling without arguments)
2. Your loop goes to i <= strlen(argv[1]), so you check the null terminator for being alphabetic (meaning you'll reject any keyword if you do the alphabetic testing right)
3. You pass argv[1], which is a pointer to char, to isalpha, you probably meant isalpha(argv[1][i]) (just leads to more keyword rejections, as you never check the actual characters but the pointer)
4. You don't check the result of get_string, on empty input it returns NULL (segfault on empty input)
5. You seem to subtract 65 or 97 both in the main programme but also in argument_lower and argument_upper
6. Those two functions seem to assume that key and plaintext character are of same case

Not sure whether your segfault was in the list (you can identify the line using debug50), but you should restructure your code a bit, especially DRY - Don't Repeat Yourself (the increment of arg_index could be at the end of the if(plain_alpha) block, also arg_index -= strlen(argument) in this case is same as arg_index = 0, or you'd use arg_index = (arg_index + 1) % strlen(argument); for incrementing and wrapping around)

I used like key = toupper(argument[arg_index]) - 'A';, and that way ignored the whole upper/lowercase for the key (but not for the plaintext character).

BTW, do not use == true, as some functions return values other than 1 (the value of true) and still mean true. Just use if (plain_alpha), which is about the same as if (plain_alpha != false). false does not have that problem, as the only value taken for false is 0, which is its actual value.