C source code error in Caesar cipher

Question

I've been trying to fix this source code for a long time but the compiler still shows error.

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

int main(int argc, char* argv[])
{
    char ptext[40];
    int i=0;
    if(argc!=2)
    {
        printf("invalid key");
        return 1;
    }
    else
        printf("enter plain text\n");
    ptext= GetString();
    int key= atoi(argv[1]);
    int n=strlen(ptext);
    while( ptext[i]!= '\0')
    {
        if( ptext[i]>65 && ptext[i]<90)
        {
            int c= (ptext+key)%26;
            int d= c+26;
            printf("%c", d);
        }
        else if( ptext[i]>97 && ptext[i]<122)
        {
            int c= (ptext+key)%26;
            int d= c+26;
            printf("%c", d);
        }
        else
        {
            printf("%c",ptext[i]);
        }
        i++;
    }
}

The errors it shows while compiling are array type 'char [40]' is not assignable (which does nothing even when a number smaller than 40 is entered or the brackets are left empty), and invalid operands to binary operation int c = (ptext+key)%26.

`char ptext[40];` should be `char *ptext;` or `string ptext;` — BLUEPIXY, Aug 21 '14 at 17:44
Is this C or C++? idomatic answers here should be different. — crashmstr, Aug 21 '14 at 17:48
First, there is no way this would compile using C, since you declare variables after executable lines. That is not allowed in C programming. So are you compiling this as a 'C' module or C++ module? I am referring to this piece of code: `ptext= GetString(); int key= atoi(argv[1]); int n=strlen(ptext);` — PaulMcKenzie, Aug 21 '14 at 17:56
@PaulMcKenzie: [That hasn't been true for 15 years](http://stackoverflow.com/a/13103673/560648), being a restriction in C89 that was removed by C99. Still, yes, this should be tagged C _or_ C++. — Lightness Races in Orbit, Aug 21 '14 at 18:00
@LightnessRacesinOrbit - Haven't done any coding using C99, so good to know. — PaulMcKenzie, Aug 21 '14 at 18:09
note: `int d= c+26;` is certainly amiss. Maybe `int d= ((c-'a')+26)%26 + 'a';` — chux - Reinstate Monica, Aug 21 '14 at 18:20
@BLUEPIXY is rigtht `char ptext[40]; ... ptext= GetString();` is bad. Maybe `char ptext[40]; ... strcpy(ptext, GetString());` — chux - Reinstate Monica, Aug 21 '14 at 18:22
@LightnessRacesinOrbit - All of my C coding (which amounts to 1% of the codebase I deal with now) is done using Visual Studio 2008, and this version was only C89 compliant. — PaulMcKenzie, Aug 23 '14 at 17:15

score 1 · Answer 1 · answered Aug 21 '14 at 19:34

1

Couple of things:

What you really need ptext to be is a char*, not an array. I'm guessing GetString() will return a string, or a char[]/char* what have you. Depending on how GetString() works, you might need to test for a NULL return.

At the line int c= (ptext+key)%26; it appears you're trying to process ptext[i]. Perhaps you forgot to include the indexer?

And just a suggestion, if using ASCII, at lines like if( ptext[i]>65 && ptext[i]<90) you can use the char values themselves instead of numbers so you don't have to pore over the ASCII tables. You can do it like so: if(ptext[i] >= 'A' && ptext[i] <= 'Z').

Notice above, also, I changed the comparators > and < to >= and <=. Are not 'A' and 'Z' valid characters?

When using the modulo as you are, for a Caesar Cipher, you want the index of the character in the alphabet. So in the case it's a capital letter, you want to subtract 'A' (or as you so aptly know, 65 in ASCII) like so : `int c= (ptext[i] - 'A' + key) % 26;'.

answered Aug 21 '14 at 19:34

Gutblender

1,340
1
12
25

1

Certainly you mean to add `'A'` back with `int c = (ptext[i] - 'A' + key) % 26 + 'A'; printf("%c", c);` – chux - Reinstate Monica Aug 21 '14 at 20:01
Yes, thank you, or in the next line with `int d = c + 'A';`. Indeed that's another fix that needs to happen. Indeed I say remove the declaration of `d` entirely as it's useless. – Gutblender Aug 21 '14 at 20:08
3

You can use `'A'` whether or not you are using ASCII , and it makes the code easier to read in both cases. (However `'A'` to `'Z'` are not consecutive in some other character sets!) – M.M Aug 21 '14 at 20:18
Veering OT, but still: for a nonconsecutive encoding - not only useful for weird encodings such as [EBCDIC](http://en.m.wikipedia.org/wiki/EBCDIC) where `A..Z` is not, but also for alphabets which are carelessly stored out-of-order in codepages (or, indeed, in Unicode itself) -, you could create a static string containing all characters in the proper order. – Jongware Aug 21 '14 at 22:02

score 1 · Answer 2 · answered Aug 21 '14 at 20:21

1

Change char ptext[40]; to char *ptext;

The GetString() function from cs50.h dynamically allocates some memory and returns a pointer to it. In C it is not possible to assign to an array, nor to return an array.

When you have finished accessing the string's contents, do free(ptext); to release the memory used.

answered Aug 21 '14 at 20:21

M.M

138,810
21
208
365

I made the suggested changes..and it does not show any error while compiling now but shows question marks in place of every ciphered text in result alongwith segmentation fault (core dumped). what does that mean? – jigl Aug 24 '14 at 17:48
First of all learn how to debug. You should be able to find out on your own which line had the segfault and what the variables were – M.M Aug 24 '14 at 19:36

C source code error in Caesar cipher

2 Answers2