Caesar code in C extra letter for result

Question

#include <stdio.h>

void caesar(char bemenet[], char eredmeny[], int n){ 
int i = 0; 
    for(i = 0; bemenet[i] != '\0'; i++) { 
        if(bemenet[i] == 'z') { 
            eredmeny[i] = 'a'; 
            eredmeny[i] += n-1;
        } 
        else 
        {
            eredmeny[i] += n; 
        } 
    } 
    eredmeny[i] = '\0'; 
}

int main(){
char tomb1[]="caesarkodolas";
char tomb2[]="";

caesar(tomb1,tomb2,1);

printf("%s \n",tomb2);

return 0;
}

My out for the "eredmeny" (result) this: "dbftbslpepmb" but tomb2=> ☺dbftbslpepmb it's not OK.. cause I have an extra char |☺|..

The output array is too small, so this is blatant undefined behaviour. — Kerrek SB, Nov 26 '13 at 14:00
This approach is broken - it is not going to work for `n` above `1`. Try your modified code with `caesar(tomb1,tomb2,10);` [to see some garbage output](http://ideone.com/cyVmK0). — Sergey Kalinichenko, Nov 26 '13 at 14:19

Sergey Kalinichenko · Answer 1 · 2013-11-26T14:22:22.680

Allocate enough memory for the second parameter, and change this line

eredmeny[i] += n;

to this:

eredmeny[i] = bemenet[i] + n;

Note that this is not a bulletproof implementation of Caesar cipher: it would work for n==1, but it will break for larger n.

You need to think of a different way of implementing the "wrap-around": rather than testing for 'z' and replacing it with 'a', compute the new position of a letter modulo 26, and then add a to it:

void caesar(char bemenet[], char eredmeny[], int n){ 
    int i;
    for(i = 0; bemenet[i] != '\0'; i++) {
        // Compute the position of the replacement letter
        int pos = (bemenet[i] - 'a' + n) % 26;
        // Place the letter into the output.
        eredmeny[i] = 'a' + pos; 
    } 
    eredmeny[i] = '\0'; 
}

demo.

score 0 · Accepted Answer · answered Nov 26 '13 at 14:07

0

First of all you should have tomb2 big enough to store result.

For example, as mentioned above

char tomb2[255] = {0};

Also you have error here

else 
{
    eredmeny[i] += n; 
}

You have to assign valid ASCII value to eredmeny[i] so change this string to

eredmeny[i] += bemenet[i] + n

Also it usually bad practice to pass a pointer on array without passing its size. Easy to get buffer overflow.

answered Nov 26 '13 at 14:07

Deck

1,969
4
20
41

Since you accepted this answer, please look at @dasblinkenlight 's advice how to write bulletproof version of the algorithm. – Deck Nov 26 '13 at 14:19

score 0 · Answer 3 · answered Nov 26 '13 at 14:09

you're not doing the maths right.

if you are using just lower case letters then you need to add n, but then many letters will be "after" z, so you need to start again at a.

you want something more like this:

for(i = 0; bemenet[i] != '\0'; i++) { 
    int encrypted = bemenet[i] + n;
    if (encrypted > 'z') encrypted = encrypted - 'z' + 'a';
    eredmeny[i] = (char)encrypted;
}

(and also fix the output array size as described in other answers here).

Caesar code in C extra letter for result

3 Answers3