-2

I have a program that will take in a key. This key shifts the plain text that many number of characters wrapping around z and preserving capitalization. Problems only arise when the ciphertext has the incorrect sized container. Why is the ciphertext array bigger then my planetext string?

#import <cs50.h>
#import <stdio.h>
#import <math.h>
#import <string.h>
#import <ctype.h>
#import <stdlib.h>

bool is_number(string str);
void print_string(string call, string s);

// Your program must accept a single command-line argument, a non-negative integer. Let’s call it k for the sake of discussion.
int main(int argc, string argv[])
{
    //return error because there was no key given
    if (argc == 1 || !is_number(argv[1]))
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }
    else if (argc != 2){
        printf("Usage: ./caesar key\n");
        return 1;
    }

    string plaintext = get_string("plaintext:");
    char ciphertext[strlen(plaintext)];
    int k = atoll(argv[1]) % 26;
    printf("plain Text: %s\n", plaintext);
    printf("Text len: %lu\n", strlen(plaintext));
    printf("Char len: %lu\n", strlen(ciphertext));
    printf("Char 1: %c\n", ciphertext[0]);
    printf("key length :%i\n", k);
    printf("ciphertext before: %s\n", ciphertext);
    
    for (int i = 0, n = strlen(plaintext); i < n; i++)
    {
        //isupper
        if (isupper(plaintext[i]))
        {
            //does it go past Z?
            if (plaintext[i] + k > 'Z')
            {
                ciphertext[i] = plaintext[i] + k - 'Z' + 'A' - 1;
            }
            // does it not go past Z
            else
            {
                ciphertext[i] = plaintext[i] + k;
            }
        }
        //is lower
        else if (islower(plaintext[i]))
        {
            //does it go past z?
            if (plaintext[i] + k > 'z')
            {
                ciphertext[i] = plaintext[i] + k - 'z' + 'a' - 1;
            }
            // does it not go past z
            else
            {
                ciphertext[i] = plaintext[i] + k;
            }
        }
        // if anything else don't change it
        else
        {
            ciphertext[i] = plaintext[i];
        }
    }
    printf("ciphertext after: %s", ciphertext);
    printf("\n");
    return 0;
}

The scrambling of the text works just fine. I just don't understand why I'm having some garbage values at the end of some of the unit tests.

This is the output for my code:

plaintext:a
Plane Text: a
Text len: 1
Char len: 6
Char 1: 
    key length :1 //this is not a tabbing error. This was my output.
ciphertext:b*

plaintext:hello
plain Text: hello
Text len: 5
Char len: 6
Char 1: 
key length :12
ciphertext before: n2
ciphertext after: tqxxa

plaintext:asdfjdnghsidkwqd
plain Text: asdfjdnghsidkwqd
Text len: 16
Char len: 6
Char 1:  
key length :12
ciphertext before:  $
ciphertext after: meprvpzsteupwicp

plaintext:ashdngkdirheknshd
plain Text: ashdngkdirheknshd
Text len: 17
Char len: 0
Char 1: 
key length :12
ciphertext before: 
ciphertext after: metpzswpudtqwzetp'

The thing I notice is that char len is 6 until I put in over 16 characters in my plaintext string. Then it goes down to 0. I assume my problem is here somewhere but I don't know enough about computer science to figure out what is going on. Can you enlighten me?

Funlamb
  • 551
  • 7
  • 20
  • 1
    You're calling `strlen(ciphertext)` before you ever fill in `ciphertext`. – Barmar Aug 14 '20 at 20:55
  • 2
    Is `string` a `typedef` in your environment? – Stephen Newell Aug 14 '20 at 20:55
  • @StephenNewell It's obviously CS50. – Barmar Aug 14 '20 at 20:56
  • You're also trying to print `ciphertext[0]` before you initialized it. That's why you get the weird tabbing. – Barmar Aug 14 '20 at 20:58
  • 3
    Are you using C or C++? You have the question tagged C++, but the code looks like C. – Barmar Aug 14 '20 at 20:59
  • The shown code is very likely to be fake code and not even real code. Setting aside the issue of variable-length arrays being non-standard C++, `plaintext` is not even defined anywhere. – Sam Varshavchik Aug 14 '20 at 21:03
  • @SamVarshavchik Obviously `planetext` was meant to be `plaintext` – Remy Lebeau Aug 14 '20 at 21:05
  • And obviously even if so, a `strlen` of either `planetext` nor `plaintext` will be accepted by any self-respecting C++ compiler. Maybe MSVC compiler, perhaps, as a result of Microsoft's ever-ending quest to embrace and extend C++, in this case by overloading `strlen` for a `std::string`. But that's unlikely, and I have a serious lack of imagination, and willingness to speculate as to what the real code actually is, and what the theoretical issue is. – Sam Varshavchik Aug 14 '20 at 21:09
  • What is `string`? Is there a `typdef` somewhere? Or is there a `using namespace std;`? You need to give us enough code to replicate the problem. You need to remove code not necessary to replicate the problem. – David Schwartz Aug 15 '20 at 00:01
  • @Barmar I knew I was printing ciphertext before I filled it. I wanted to see what value was in there before I added to it. I was assuming there was some type of garbage value in there messing up my program. – Funlamb Aug 15 '20 at 00:05
  • Accessing uninitialized variables causes undefined behavior. – Barmar Aug 15 '20 at 00:55
  • @Barmar I figured that much. I am trying to fill 'ciphertext' after creating it. I'm doing to wrong somehow. I try to create it with the correct size but it will not be the same length as 'plaintext'. That is where I started looking. Do I need to do something else to make 'ciphertext' the correct size? – Funlamb Aug 15 '20 at 01:07
  • There are many questions about caesar cipher, as well as wikipedia and other sites. – Barmar Aug 15 '20 at 01:08
  • @Funlamb Breaking out that part of the problem to make a [mcve] shouldn't be hard. It'll be 5 lines of code tops. – Ted Lyngmo Aug 15 '20 at 01:09
  • My problem isn't with the cipher. My problem is that when I print 'ciphertext' I have some garbage value at the end that is causing my unit tests to fail. The output is almost correct except for the garbage at the end of 'ciphertext'. That is where I am strugeling. – Funlamb Aug 15 '20 at 01:13
  • @Funlamb Don't the answers you've gotten explain that part? – Ted Lyngmo Aug 15 '20 at 06:55
  • @TedLyngmo I tried adding the null character to ciphertext but it didn't work. – Funlamb Aug 16 '20 at 23:53
  • @Funlamb Ok, then provide a [mcve]. There are some function definitions missing in the code you've posted, like `get_string`. The header `cs50.h` isn't standard. What does it contain? Also, `#import` isn't a standard pre-processor directive. Use `#include`. – Ted Lyngmo Aug 17 '20 at 05:19

2 Answers2

1

You need to add one to strlen to allow room for the '\0' that terminates the string.

Charlie Martin
  • 110,348
  • 25
  • 193
  • 263
1

char ciphertext[strlen(plaintext)]; is a variable-length array, which is non-standard in C++, see Why aren't variable-length arrays part of the C++ standard?.

In any case, that declaration does not provide space for a null terminator. You need to add +1 for that:

char ciphertext[strlen(plaintext)+1];

And then, make sure the last unused char of the ciphertext is actually set to '\0', eg:

// copy some text into ciphertext, then...
ciphertext[LengthActuallyUsed] = '\0';

For that matter, you are not populating ciphertext with any text data before trying to print it, so printf() will print garbage (if anything at all) since ciphertext is not guaranteed to be null-terminated in its declaration.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770