Vigenere-cipher wrong output

Question

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);
}

Answer 1

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];
}