iced iced - 3 months ago 10
C Question

I have no idea why I'm receiving segmentation fault errors

#include <stdio.h>
#include <cs50.h> //stdlib.h is included in cs50.h so I don't need it
#include <string.h>
#include <ctype.h>
#include <math.h>

int main(int argc, string argv[]) // command line input
{
if(argc != 2) // check if there is only one input
{
printf("error\n");
return 1;
}

int commandlength = strlen(argv[1]); // find string length of command string

string key[commandlength + 1]; // taking the key from the input and putting it in something that will take less typing later

for(int i = 0; i < commandlength; i++) // check if every char in the string is a letter
{
if(isalpha(argv[1][i]))
continue;

else
{
printf("error\n");
return 1;
}
}

strcpy(key[commandlength], argv[1]); // copy key from command line into a string called key
string input = GetString();
int inputlength = strlen(input); // length of string typed in when prompted
int k = 0; // this will be used to iterate the key separately from i, since the key only iterates when applied to an alpha

for(int i = 0; i < inputlength; i++)
{
if(ispunct(input[i]))
printf("%c", input[i]);

if(isspace(input[i]))
printf("%c", input[i]);

if(isupper(input[i]))
{
printf("%c", (input[i] + atoi(key[k]) % commandlength - 65) % 26 + 65);
k++;
}

if(islower(input[i]))
{
printf("%c", (input[i] + atoi(key[k]) % commandlength - 97) % 26 + 97);
k++;
}
}

printf("\n");
return 0;
}


Before reading, please keep in mind that I am a student. It is much more beneficial to receive a detailed explanation than simply a line of code saying "here you go I fixed it". I will be linking this post in my submission, and we do have very strict rules about academic integrity and what is copying vs helping, and I'd really appreciate if that were kept in mind.

The purpose of this project is to create a vigenere cipher. Below are the instructions my teacher provided:

Your final challenge this week is to write, in vigenere.c, a program that encrypts messages using Vigenère’s cipher. This program must accept a single command-line argument: a keyword, k, composed entirely of alphabetical characters. If your program is executed without any command-line arguments, with more than one command-line argument, or with one command-line argument that contains any non-alphabetical character, your program should complain and exit immediately, with main returning 1 (thereby signifying an error that our own tests can detect). Otherwise, your program must proceed to prompt the user for a string of plaintext, p, which it must then encrypt according to Vigenère’s cipher with k, ultimately printing the result and exiting, with main returning 0.

As for the characters in k, you must treat A and a as 0, B and b as 1, … , and Z and z as 25. In addition, your program must only apply Vigenère’s cipher to a character in p if that character is a letter. All other characters (numbers, symbols, spaces, punctuation marks, etc.) must be outputted unchanged. Moreover, if your code is about to apply the jth character of k to the ith character of p, but the latter proves to be a non-alphabetical character, you must wait to apply that jth character of k to the next alphabetical character in p; you must not yet advance to the next character in k. Finally, your program must preserve the case of each letter in p.

I'm pretty sure I hit the requirements for iterating through the code, making sure the key loops if it's shorter than the phrase being encrypted, and making sure that I'm not applying the key to non-alpha characters. However, I get a segmentation default before prompted to enter a string to encrypt, so I'm pretty sure the problem lies in the top half of the code. I've looked over the code quite a bit and I still can't figure out why it keeps segfaulting.

Answer

Change this statement

strcpy(key[commandlength], argv[1]); 

to

strcpy( key, argv[1] ); 

key[commandlength] is an object of type char value of which is used as an address of a string if you use this invalid construction

strcpy(key[commandlength], argv[1]); 

Also it is not a good idea to use continue in your loop. Instead of

for(int i = 0; i < commandlength; i++) // check if every char in the string is a letter
{
    if(isalpha(argv[1][i]))
        continue;

    else
    {
        printf("error\n");
        return 1;
    }
}

I would write simply

for(int i = 0; i < commandlength; i++) // check if every char in the string is a letter
{
    if( !isalpha(argv[1][i] ) )
    {
        printf("error\n");
        return 1;
    }
}

And as BLUEPIXY noted instead of

string key[commandlength + 1]; 

there shall be

char key[commandlength + 1]; 

provided that you have no some typedef similar to

typedef char string;

However if you indeed has such a typedef then the following statement

string input = GetString();

will be invalid because you are trying to call function strlen for input:

int inputlength = strlen(input); 

And do not use magic numbers in your code as for example in this statement

printf("%c", (input[i] + atoi(key[k]) % commandlength - 65) % 26 + 65);

I suppose that 65 is 'A'. So it would be better to use explicitly character literal 'A' instead of 65.