Reuben John Reuben John - 3 months ago 9
C Question

segmentation fault in c while coding Vigenère’s cipher

I am really new to coding and I have been teaching myself how to code by using EDX.org. This week I have been studying about Cryptography and I have to create a Vigenère’s cipher. I wrote the code and for the most part, it's correct. However when I compiled the program, it's showing a segmentation error. I have been trying to figure our why this is happening and I am totally stuck. Could you take a look at my code and tell me whats wrong?

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


int index(int k, int c);

int main (int argc, string argv[1])
{
//check for correct criteria
if (argc = 2, isalpha(argv[1]))
{
string text = GetString();
string key = argv[1]; //store key word
int Totalshift = strlen(key); //number of shift for keyword


int shift = 0;

//loops over the whole text
for (int i = 0, n = strlen(text); i <n; i++ )
{
char p= text[i];
char k = toupper(key[shift]); //Upper case for each character

if (isupper(p))
{
//convert to 0 index
p = p - 65;
k = k - 65;

int crypt= index (k , p);

printf("%c", crypt+65);

shift= (shift+1) %Totalshift;
}
else if (islower(p))
{

p = p - 97;
k = k - 65;

int crypt= index (k , p);

printf("%c", crypt+97);

shift= (shift+1) %Totalshift;
}
else
{
printf("%c", p);
}


}
printf("\n");
}




//error message
else
{
printf("ERROR!\n");
return 1;
}


}

//index function
int index(int k, int p)
{
return (k+p)% 26;

}

Answer

string

No. Never, ever hide pointers.

int main(int argc, char ** argv)

Then:

//check for correct criteria 
if (argc = 2, isalpha(argv[1]))

Here, you assign to the variable (parameters behave like local variables in that respect) the value 2, thus destroying the previous value (which holds the number of arguments given to the program). The result is the value which was assigned, thus 2. Then, there's the comma operator: You discard that 2, and then call isalpha(argv[1]), which clearly shows why you should always turn on warnings and never, ever hide pointers:

argv[1] is of type char *, thus a pointer to a character array (or, as we know in this case, a character array terminated with '\0', which is called a C string). Since isalpha takes an int as parameter the value of the pointer ("the memory address") is implicitly converted to (a probably very large) int value. Quoting from above link, emphasis mine:

The c argument is an int, the value of which the application shall ensure is representable as an unsigned char or equal to the value of the macro EOF. If the argument has any other value, the behavior is undefined.

Which is possibly the source of the segmentation fault.

Finally, that GetString really looks fishy to me. Assuming that it allocates some memory (for the string that it presumably reads from the user) ... where do you free that memory? Is it really allocating memory, or possibly returning a pointer to an array with automatic storage duration (a local variable, so to say)?