0

OK, I'm really confused here. My pset2 Vigenere is failing the tests but when I look at the output from the test the expected outputs are exactly the same as the actual outputs. I have tried adding a \n at the end of the output and also tried removing it but nothing seems to work. Any ideas what I'm missing? I desperately want to start cleaning my code up and move on but I can't until I get he outline working.

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

// variable declarations

if (argc != 2){
    printf("No key value was input");
    return 1; 
    }   

string key = "";
int i = 0;
int j = 0;
int q = 0;
int keylen = 0;
string plaintext = "";
int plainlen = 0;

key = argv[1];
keylen = strlen(key);

for(i = 0; i <keylen; i++){
    //convert all key to lower case
    if((key[i] < 65 || key[i] > 122) || (key[i] > 90 && key[i] < 97)){
        printf("None alphabetical character detected. Exiting...");
        return 1;  }
    key[i] = tolower(key[i]);
    key[i] = key[i]-97;
    }

printf("plaintext: ");
plaintext = get_string();   
plainlen = strlen(plaintext);
printf("ciphertext: ");

for(i = 0; i < plainlen; i++){
    if(q > plainlen){
            break;
        }
    // outer text loop
    for(j = 0; j < keylen; j++){
        if(q > plainlen){
            break;
        }
        // inner text loop

        // if we have non-alphabetic
        if((plaintext[q] < 65 || plaintext[q] > 122) || (plaintext[q] > 90 && plaintext[q] < 97)){
            printf("%c", plaintext[q]);
            //can we go negative here???
            j--;
            q++;
        }
        // check if it's wrapped
        else if(((plaintext[q]+(key[j]) > 90 && toupper(plaintext[q]) == plaintext[q])) || ((plaintext[q]+(key[j]) > 122 && tolower(plaintext[q]) == plaintext[q]))){
            printf("%c", (plaintext[q]+key[j]-26));
            q++;
        }
        // normal shift
        else{
            printf("%c", (plaintext[q]+key[j]));
            q++;
        }

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

The output is showing:

:) vigenere.c exists.
:) vigenere.c compiles.
:) encrypts "a" as "a" using "a" as keyword
:( encrypts "barfoo" as "caqgon" using "baz" as keyword
expected "ciphertext: caq...", not "ciphertext: caq..."
:( encrypts "BaRFoo" as "CaQGon" using "BaZ" as keyword
expected "ciphertext: CaQ...", not "ciphertext: CaQ..."
:( encrypts "BARFOO" as "CAQGON" using "BAZ" as keyword
expected "ciphertext: CAQ...", not "ciphertext: CAQ..."
:( encrypts "world!$?" as "xoqmd!$?" using "baz" as keyword
expected "ciphertext: xoq...", not "ciphertext: xoq..."
:) handles lack of argv[1]
:) handles argc > 2
:) rejects "Hax0r2" as keyword

EDIT: OK, I have changed the program so it no longer copies the pointer location of argv[1] and instead uses an array of ints to store the letter score but it still has the same result of passing all the tests whilst also failing them.

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

// variable declarations

if (argc != 2){
    printf("No key value was input");
    return 1; 
    }   

int keylen = strlen(argv[1]);
int key [keylen];
int i = 0;
int j = 0;
int q = 0;
string plaintext = "";
int plainlen = 0;

for(i = 0; i < keylen; i++){
    //convert all key to lower case
    if (argv[1][i] >= 'A' && argv[1][i] <= 'Z'){
        key[i] = argv[1][i]-65;
    }
    else if (argv[1][i] >= 'a' && argv[1][i] <= 'z'){
        key[i] = argv[1][i]-97;
    }
    else{
        printf("None alphabetic character detected in key. Exiting...");
        return 1;
    }
    }

printf("plaintext: ");
plaintext = get_string();   
plainlen = strlen(plaintext);
printf("ciphertext: ");

for(i = 0; i < plainlen; i++){
    if(q > plainlen){
            break;
        }
    // outer text loop
    for(j = 0; j < keylen; j++){
        if(q > plainlen){
            break;
        }
        // inner text loop

        // if we have non-alphabetic
        if((plaintext[q] < 65 || plaintext[q] > 122) || (plaintext[q] > 90 && plaintext[q] < 97)){
            printf("%c", plaintext[q]);
            j--;
            q++;
        }
        // check if it's wrapped
        else if(((plaintext[q]+(key[j]) > 90 && toupper(plaintext[q]) == plaintext[q])) || ((plaintext[q]+(key[j]) > 122 && tolower(plaintext[q]) == plaintext[q]))){
            printf("%c", (plaintext[q]+key[j]-26));
            q++;
        }
        // normal shift
        else{
            printf("%c", (plaintext[q]+key[j]));
            q++;
        }

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

}

1

You are modifying argv[1] which is not allowed. It's only luck that your program seems to work and doesn't crash. But check50 is hitting the problem.


added this bit to explain

argv[1] is a pointer to a piece of memory that holds the "string". When you then say key = argv[1]; you aren't copying the value of that string into the variable key. You are simply pointing key to that same piece of memory that argv[1] points to. So when you then change key[i] you are changing argv[1][i], which is not allowed.


If you want to have an array of ints to represent your key, you should declare it that way, say int keyints[keylen]; and then fill it using the chars in argv[1] modified to the 0-25 range as you have done directly.


2nd edit

To see where this is failing, let's look at the example of plaintext = "abc" and key = "aa".

plainlen is 3. keylen is 2.

for(i = 0; i < 3; i++)
{
    if(q > 3)
        break;

    // outer text loop
    for(j = 0; j < 2; j++)
    {
        if(q > 3)
            break;

        if we have non-alphabetic  (we don't so ignore)
        else if check if it's wrapped (it won't be so ignore)

        else
        {
            printf("%c", (plaintext[q]+key[j]));
            q++;
        }

    }
}

So now walk through the example.

q = 0

for i = 0
    if 0>3  .. it's not

    for j = 0
        if 0>3 it's not
        print plaintext[0]+key[0] //print 'a'
        q++ (1)

    for j = 1
        if 1>3  it's not so continue
        print plaintext[1]+key[1] // print 'b'
        q++ (2)

    j = 2 so that loops ends

for i = 1
    if 2>3  it's not

    for j = 0
        if 2>3 it's not
        print plaintext[2]+key[0] //print 'c'
        q++ (3)

    for j = 1
        if 3>3  it's not
        print plaintext[3]+key[1] // print (nullchar)
        q++ (4)

    for j = 2 so loop ends

for i = 2
    if 4>3 it is BREAK

So what has happened? You've printed "abc\0" Can you see why? You never want to end up printing plaintext[3] if plainlen = 3, so shouldn't you break out of the loops if q = 3 in this case (ie, if q >= plainlen)?

8
  • i don't understand at which point I'm modifying argv[1]. I pass it to another variable but at no point do I modify what's in arvg[1] unless I'm missing something. Could you please clarify? Thanks Aug 20 '17 at 6:34
  • The answer involves pointers, which isn't covered in the class until week 4 or so. I've updated my answer to address that briefly.
    – curiouskiwi
    Aug 20 '17 at 6:40
  • Ah right, I understand where I'm editing argv[1] now. So, if I use int keyints[1] = argv[1][1], would this access the necessary character in the argv pointer and add it as an int to my array of ints? Also, how do I see the length of argv[1] for keylen without passing it as a string and using strlen?? Aug 20 '17 at 6:43
  • Hi I have done (what I interpreted) as you said but still have the exact same problem. I now have int keylen = strlen(argv[1]); int key [keylen]; to get the length of the key and a blank array of the length and I use key[i] = argv[1][i]-65; or key[i] = argv[1][i]-97; to fill the array depending on whether it's upper or lower case. I assume this is still changing argv[1] and the problem is the same as last time. Aug 20 '17 at 7:31
  • The issue is that you are printing the NULL char at the end of the plaintext string. It's a non-printable value so you don't actually see it. So if your plaintext is AAA and your key is A, you are printing AAA\0.
    – curiouskiwi
    Aug 20 '17 at 9:21

You must log in to answer this question.

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