0

I instituted code to prevent any wrap around issues, I've made sure that my alphabet position calculations don't have the possibility of running past 127 and becoming negative, and even have code to determine the case of the keyword letter to avoid weird issues where 'A' would be subtracted from 'a', and yet I still somehow get incorrect results. As usual I expect the problem is something obvious that I'm missing, but I'm just not seeing it despite hours of testing, using printf's to check all my variables, checking other issues on the cs50 exchange to see if I was making one of those mistakes, etc.

It also dumps the core with a segmentation fault when no argument is given despite code being in place to prevent the user from doing so or inputting a non alphabetical key.

As always, thank you for pointing me in the right direction with my code, as it becomes a bit frustrating to research every little part of your code and not find any readily apparent reason for it's dysfunction.

int main(int argc, char* argv[])
{
for (int i = 0 ; i < strlen(argv[1]) ; i++)  
{  
    // Ensures the user inputs only a single arguement
    if (argc != 2 || !isalpha(argv[1][i]))
    {
        printf("Please input a single alphabetical keyword as an arguement.\n");
        return 1;
    }        
}

// Requests string to be converted
string plaintext = GetString();

// Determine key letter to be used
int keycharno = 0;

// Prototype
char keyposfinder;

for (int i = 0, n = strlen(plaintext); i < n; i++)
{        
    // Determine keyword char case
    if (isupper(argv[1][keycharno]))
    {
        keyposfinder = 'A';
    }
    else
    {
        keyposfinder = 'a';
    }

    // Is the letter part of alphabet
    if (isalpha(plaintext[i]) != 0)
    {

        // Determine if lowercase, print lower char
        if ((int) plaintext[i] > 96)
        {
            char ciphertext = 'a' + (((plaintext[i] - 'a')) + (argv[1][keycharno] - keyposfinder) % 26);
            printf("%c", ciphertext);
            keycharno++;
            if (keycharno > strlen(argv[1]))
            {
                keycharno = 0;
            }
        }
        // Print upper char
        else
        {
            char ciphertext = 'A' + (((plaintext[i] - 'A')) + (argv[1][keycharno] - keyposfinder) % 26);
            printf("%c", ciphertext);
            keycharno++;
            if (keycharno > strlen(argv[1]))
            {
                keycharno = 0;
            }
        }
    }
    else
    {
        printf("%c", plaintext[i]);
    }

Check50 Results:

:) vigenere.c exists
:) vigenere.c compiles
:) encrypts "a" as "a" using "a" as keyword
:( encrypts "world, say hello!" as "xoqmd, rby gflkp!" using "baz" as keyword
   \ expected output, but not "xo��Ye, szf ie��Yp!\n"
:( encrypts "BaRFoo" as "CaQGon" using "BaZ" as keyword
   \ expected output, but not "Cak3po\n"
:( encrypts "BARFOO" as "CAQGON" using "BAZ" as keyword
   \ expected output, but not "CAk3PO\n"
:( handles lack of argv[1]
   \ expected output, not standard error of "/opt/sandbox50/bin/run.sh: line 31:  85..."
:) handles argc > 2
:) rejects "Hax0r2" as keyword
1

Well, you thought you checked for all those problems, and even made a good effort, but not quite. ;-)

Let's start with the 'no key' problem. Look at this:

for (int i = 0 ; i < strlen(argv[1]) ; i++) 

If there was no key, then argv[1] == NULL. The first thing you do with argv[1] is strlen(). strlen(NULL) produces a seg fault. You should consider separating the test for argc and the for loop for alphas.

Next, look at this:

if (keycharno > strlen(argv[1]))

Now, let's say your key is BAZ. That's a key with length 3, meaning argv[] has elements 0, 1 and 2. Your test will allow keycharno to be 0, 1, 2, or 3. oops. That's what is causing the translation to get out of synch with the plaintext, along with some other issues.

You also tried valiantly to avoid the 128+ problem, but you just missed. Check the formula you used.

char ciphertext = 'A' + (((plaintext[i] - 'A')) + 
                  (argv[1][keycharno] - keyposfinder) % 26);

Examine the parentheses very carefully to see what is done in what order. In short, you are applying the % 26 operation to key - 'a' only, and not to plaintext + key converted to numbers between 0 and 26. I'll leave it to you to figure out the very simple fix to this. ;-)

Bugs like these are perfect examples of where it is very useful to insert a few extra temporary printf() statements in the code to debug, or to learn how to use gdb to debug your programs (more on gdb later in the course). For instance, you could have inserted printf("keycharno = %s\n",keycharno); to see what it was doing. These techniques will come with practice and time.

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

2
  • Once again, thank you for helping me correct my code Cliff B. Your assistance is much appreciated. I can't believe my mistakes were as simple as going one letter too far on the key and forgetting a set of brackets. (Which is astonishing because I sat and used that formula on paper to make sure that it was working correct. Oops) Anyways. I am somewhat wondering why the initial loop with the Or test for argc and alpha doesn't work when it functions perfectly when separated. That seems a bit unintuitive. Sep 9 '15 at 8:43
  • The test would work, if the code ever got that far. By itself, the test would detect the absence of a key successfully. But it never gets that far. It hits the FOR loop setup first and crashes on that statement because of the absence of a key.
    – Cliff B
    Sep 9 '15 at 14:21

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .