-2

I have to program Vigenere cipher, but my output looks a bit diffrent.

Input: Po treti raz sa ohlasi The key: euhwa

Output: TI ANEXC YWZ WU VDLEMP What I got: TI ANEDM LHV SK SBSWSS

Could you please help me to find out, why doesn't it work correctly? 

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

char* vigenere_encrypt(const char* key, const char* text) 
{
    if(key==NULL || text==NULL)
        return NULL;

    int i,k,t,j=0;
    t = strlen(text);
    k = strlen(key);

    char* copy=(char*)malloc(strlen(text)+1);
    char* enc=(char*)malloc(strlen(text)+1);
    char* copykey=(char*)malloc(strlen(key)+1);

    for (i=0;i<k;i++)
    {
        copykey[i]=toupper(key[i]);
    }

    for (i=0;i<k;i++)
    {
        if(!(isalpha(copykey[i])))
        {
            free(copy);
            free(copykey);
            free(enc);
            return NULL;
        }
    }

    for (i=0;i<=t;i++)
    {
        copy[i]=toupper(text[i]);
    }

    for (i=0;i<=t;i++)
    {
        if (isupper(copy[i]))
        {
            enc[i]=(copy[i]+copykey[j])%26+'A';
            j++;
            if (j>k)
                j=0;
        }
        else enc[i]=copy[i];
    }

    free(copy);
    free(copykey);
    return enc;
}

int main()
{
    char* encrypted;
    encrypted = vigenere_encrypt("euhwa","Po treti raz sa ohlasi!");
    printf("%s\n",encrypted);
    free(encrypted);
}
fredmaggiowski
  • 2,103
  • 2
  • 23
  • 42
Kingusss12
  • 23
  • 3
  • 2
    This code is horrendous to look at so far. Just as a general tip, add spacing, and try not to make a line like this: `for (i = 0; i < 10; i++)` look like this: `for(i=1;<10;i++)`. Also, why are you doing some things two or more times? You are looping from `i = 0` to `t` three separate times. The first two are doing the exact same thing. The more you clean up your logic, the easier it will be for YOU to debug your issue. – Ricky Mutschlechner Mar 27 '16 at 17:56
  • Also, since you're definitely doing this for CS50 (http://cs50.tv), you should not expect people to give you the answer. You should let us know what you have tried and you should make it very clear that this is a homework assignment. Also: `int i,k,t,j=0;` probably doesn't do what you think. And then you're iterating from i = 0 to `t`, which is undefined! – Ricky Mutschlechner Mar 27 '16 at 18:01
  • HINT: try adding a `printf` in the cipher loop that let you see what `char` you're using as `plaintext` and `key` because there's an evident issue in how you manage the key.. (i took your code and added this line `printf("char: %c + %c mod26 + A: %c \n", copy[i], copykey[j], enc[i]);` ) – fredmaggiowski Mar 27 '16 at 18:05
  • I'm really sorry. I copied this part: `"for (i=0;i<=t;i++){ copy[i]=toupper(text[i]);}"` 2 times. I will take care next time. Sorry for the trouble. I'm not doing this for CS50. – Kingusss12 Mar 27 '16 at 18:12

1 Answers1

0

The issue is how you handle the:

"Hey, i've finished the key, lets bring j back to zero"

Now, the code is pretty messy, not for the double copy/paste (it can happen) but because it could (and should) be optimised a little..

Anyhow the solution for your issue is simple:

you should set j to zero when j is >= of k. it is important the equal part since you want it to be 0 when you reach the length of the key. The way you are doing it (only testing for greater than) means that when you reach the end of the key you do an extra loop that uses an invalid copykey value.. if the key has length equal to 5 you should stop at copykey[4] but the extra loop does copykey[5] which is invalid.. and (un)luckily it doesn't segfault you to hell.

for (i=0;i<=t;i++)
{
    if (isupper(copy[i]))
    {
        enc[i]=(copy[i]+copykey[j])%26+'A';
        j++;
        if (j >= k)
            j=0;
    }
    else enc[i]=copy[i];
}
fredmaggiowski
  • 2,103
  • 2
  • 23
  • 42