My vigenère is spitting out random strings. For example, here is a test I got from my terminal

~/pset2/ $ ./vigenere a    
plaintext:  a
ciphertext: G
~/pset2/ $ 

And my code is here. I suspect that the bug is somewhere in the shift function.


There are a number of issues in the code that are impacting it.

First, look at this:

int *keyInt = malloc(strlen(key)); 

This allocates n bytes where n is the number of chars in the key string. However, the pointer keyInt is a pointer to a single integer. Even with that, a char is one byte, an int is 4 bytes, so if you're converting a char to an int, you'd need 4 times the space allocation.

But why convert? A char behaves and can be treated like it is a one byte signed int.

Next, the sizeof(pointer) is always 8, it isn't the length of a string. Use strlen().

The length of string to be processed is the plaintext. The length of the key is used to determine when to restart with the beginning of the string.

There needs to be two index values to track position of the current letter in the key and in the plaintext. They cannot be the same index and they can't be related to each other.

When subtracting from the plaintext or key letter to set them to a number between 0 and 25 inclusive, you need to subtract 'a' or 'A', not ` or @. The latter two will result in encoding from 1 to 26, rendering the use of % invalid.

At this point, I stopped looking. There are just too many issues that need to be fixed, and the logic is far more complex than needed.

Kudos for practicing how to write and call functions, but there are far too many functions in this code. When a function is called only once, it's a red flag that a function is not needed and that the code should just be written where the function is called from. Most of these functions are chained together, another red flag. Code that looks suspiciously like existing library functions, like toupper(), is yet another. Instead of a function, just inline the library function call in a for loop.

Don't make code overly complex. KISS - Keep It Simple!

There is a lot of rethink needed here, but this should get you started.

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)

  • I took a bit of your advice, and it worked!!! πŸŽ‰πŸŽŠπŸŽ‰βœ¨πŸŽ‰πŸŽŠπŸŽ‰ – Mayonnaise2124 Apr 6 '19 at 1:10
  • A bit of my advice, huh? Did I just hear a rimshot? ;-) – Cliff B Apr 6 '19 at 8:07

You must log in to answer this question.

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