0

What's happening is: the code works through the iterations equal to the length of 'key', after that, it starts generating a single value over and over that seems to be double what it should be. I tried inserting printf alerts (which I've now taken out) in order to try to debug this on my own, but I'm not finding anything out of line.

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


int main(int argc, string argv[])
{


    if(argc > 2 || argc < 2 ) //check to make sure only one number was given as an argument
    {
        printf("Program requires one, and only one alphabetical argument at runtime.\n");
        return 1;
    }

    char key[strlen(argv[1])];

    string plaintxt;

    int increment = 0;

    for(int i = 0; i < strlen(argv[1]); i++) //create key variable, and ensure string is only alphabetical
    {

        if( (argv[1][i] >= 65 && argv[1][i] <=90) || (argv[1][i] >= 97 && argv[1][i] <= 122) )
        {
            key[i] = argv[1][i];
        }
        else
        {
            printf("Program requires one, and only one alphabetical argument at runtime.\n");
            return 1;
        }
    }

    for(int j = 0; j < strlen(key); j++) //convert key to lower-case
    {
        key[j] = tolower(key[j]);
    }

    printf("plaintext: ");

    plaintxt = get_string(); // get plaintext input, assign it to plaintxt

    for(int i = 0; i < strlen(plaintxt); i++)
    {
        if(plaintxt[i] >= 65 && plaintxt[i] <= 90) //is a capital letter
        {
            if(plaintxt[i] + (key[increment] - 97) > 90) //keep cypher value in capital letter range
            {
                plaintxt[i] = (key[increment] - (key[increment] - 64)) + (key[increment] -97);
            }
            else // simply add the cipher value to the correct position
            {
                plaintxt[i] = plaintxt[increment] + (key[increment] - 97);
            }
            increment++;
        }

        else if(plaintxt[i] >= 97 && plaintxt[i] <= 122) // is a lower-case letter
        {
            if(plaintxt[i] + (key[increment] - 97) > 122) //keep cypher value in lower-case letter range
            {
                plaintxt[i] = (key[increment] - (key[increment] - 97)) + (key[increment] -97);
            }
            else //simply add the cipher value to the correct position
            {
                plaintxt[i] = plaintxt[increment] + (key[increment] - 97);
            }

            increment++;
        }
        else // if not a capital or lower-case, leave unchanged
        {
            plaintxt[i] = plaintxt[increment];
        }

        if(increment == strlen(key)) //roll the incrementer back down to the first letter if it's at the last
        {
            increment = 0;
        }
    }

    printf("ciphertext: %s\n", plaintxt);

    return 0;
}

thanks

1

Review strings in C. They require space for all the characters you'd like to store plus a null character '\0' marking the string end. Without, strlen(key) will read beyond the string, until it finds a zero byte or segfaults.

    char key[strlen(argv[1]) + 1];
    key[strlen(argv[1])] = '\0';

might work.

Instead of an extra loop, you could use tolower in the first loop already.

Also, make sure you use plaintxt[i], not plaintxt[increment].

And your formulae are wrong. Your formula

plaintxt[i] = (key[increment] - (key[increment] - 64)) + (key[increment] -97)

is same as

plaintxt[i] = key[increment] - 33

I used the remainder operator %, like

        if(plaintxt[i] >= 65 && plaintxt[i] <= 90) //is a capital letter
        {
            plaintxt[i] = (key[increment] - 97 + plaintxt[i] - 65) % 26 + 65;
            increment++;
        }

The %26 will wrap anything over 25 back into the range of 0..25, where (after the backshift with +65) we have our alphabet.

5
  • Thank you so much for your notes and your help. You have made my homework tonight bearable.
    – phindex
    Oct 25 '17 at 12:42
  • Hi! I finally got it to compile and check50 with no problems. Afterword I decided to mess with the code a bit to test what you were showing me about needing the '\0' character in the last position of the array. I took it out, and it still works!??! Do you know why? Is it a version of C that automatically puts that last character in? Thanks again
    – phindex
    Oct 26 '17 at 12:35
  • char key[strlen(argv[1]) /* + 1*/ ];
    – phindex
    Oct 26 '17 at 12:35
  • // key[strlen(argv[1])] = '\0';
    – phindex
    Oct 26 '17 at 12:35
  • The +1 is needed as there might be another variable just after the array (most likely with array lengths of a multiple of 8), and the value of that next variable would determine the character. About the ='\0', only global variables are zeroed automatically. Local variables contain whatever was at that place in memory. That might be zero, or something else. Usually, those cases work until they break after changing something completely unrelated. Never rely on uninitialised memory (again, global variables are explicitly zero if no initial value specified).
    – Blauelf
    Oct 26 '17 at 14:36

You must log in to answer this question.

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