CS50 Substitution

Question

This problem set requires us to code a programme that takes a key from the user from the CLI and then an input (plaintext) and return a ciphertext version that is scrambled based on the key provided.

My code returns the correct ciphertext given any key and plaintext, however, the apparent output when using the in-built check50 module from cs50 is "", an empty string.

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

string encrypt(string keys, string inputs);

char newkey;
string ciphertext;

int main(int argc, string argv[])
{
    if (argc == 1)
    {
        printf("Please enter a key!\n");
        return 1;
    }

    if (argc > 2)
    {
        printf("You can only have one key. The key must not have any spaces!\n");
        return 1;
    }

    if (0 < strlen(argv[1]) && strlen(argv[1]) < 26)
    {
        printf("Key must contain 26 characters!\n");
        return 1;
    }

    for (int j = 0; j < strlen(argv[1]); j++)
    {
        if (!((argv[1][j] >= 'a' && argv[1][j] <= 'z') || (argv[1][j] >= 'A' && argv[1][j] <= 'Z')))
        {
            printf("Key must contain alphabets only!");
            return 1;
        }
    }

    for (int k = 0; k < strlen(argv[1]); k++)
    {
        for (int l = (k + 1); l < strlen(argv[1]); l++)
        {
            if (argv[k] == argv[l])
            {
                printf("There can be no duplicate alphabets in the key!");
                return 1;
            }
        }
    }

    string key = argv[1];

    string input = get_string("plaintext: ");

    encrypt(key, input);

    printf("ciphertext: %s\n", ciphertext);

}

string encrypt(string keys, string inputs)
{
    char ciphertexts[strlen(inputs)];
    for (int i = 0; i < strlen(inputs); i++)
    {
        if (islower(inputs[i]))
        {
            int index = inputs[i] - 97;
            newkey = keys[index];
            ciphertexts[i] = tolower(newkey);
        }
        else if (isupper(inputs[i]))
        {
            int index = inputs[i] -65;
            newkey = keys[index];
            ciphertexts[i] = toupper(newkey);
        }
        else
        {
            ciphertexts[i] = inputs[i];
        }
    }
    ciphertext = ciphertexts;
    printf("%s\n",ciphertexts);
    printf("%s\n",ciphertext);
    return ciphertext;
}

The errors are as follows:

:( encrypts "A" as "Z" using ZYXWVUTSRQPONMLKJIHGFEDCBA as key
    expected "ciphertext: Z\...", not ""

This means that using a key of ZYXWVUTSRQPONMLKJIHGFEDCBA, and a plaintext of "A", "Z" is expected, but my programme outputs "".

I have a printf statement printing the cipher text, but it is somehow not captured

Answer 1

Memory management

Rather than return a pointer to a local variable (which is no longer valid after function ends @kaylum), pass into encrypt() a pointer to where to form the encrypted string.

Don't forget about '\0'

In forming the encrypted string account for the null character. @kaylum

Avoid repeated calls to strlen(inputs)

Advanced: Avoid negative ch with is...(ch)

---

Along with some other ideas, perhaps:

#include <ctype.h>

void encrypt(char *ciphertexts, const char *keys, char *inputs) {
  // Let's work with unsigned char to avoid trouble negative char
  const unsigned char *ukeys = (const unsigned char*) keys;
  const unsigned char *uinputs = (const unsigned char*) inputs;
  unsigned char *uciphertexts = (unsigned char*) ciphertexts;
  size_t i = 0;

  // Code uses a do loop to catch the null character.
  do {
    if (islower(uinputs[i])) {
      int index = uinputs[i] - 'a';
      unsigned char newkey = ukeys[index];
      uciphertexts[i] = tolower(newkey);
    } else if (isupper(uinputs[i])) {
      int index = uinputs[i] - 'A';
      unsigned char newkey = ukeys[index];
      uciphertexts[i] = toupper(newkey);
    } else {
      uciphertexts[i] = uinputs[i];
    }
  } while (uinputs[i++]);
}

Code still has trouble when the locale is not "C" or with non-ASCII, but we can leave those as advanced issues.

---

Advanced

Since strings to encrypt can be lengthy, consider pre-forming a key valid for all char. I.e.

unsigned char keys256[UCHAR_MAX + 1] = { 0 };
for (i = 0; i <= UCHAR_MAX; i++) {
  keys256[i] = i;
}
for (i = 0; key[i]; i++) {
  keys256['a' + i] = tolower(key[i]);
  keys256['A' + i] = toupper(key[i]);
}

and then in encrypt(..., keys256, ...), use a greatly simplified loop.

  ...
  do {
    uciphertexts[i] = keys256(uinputs[i]);
  } while (uinputs[i++]);
  ...