The below code runs encryption part well but when 'z' is entered it gives unexpected output and doesnt run decryption part

Question

#include<stdio.h>

const char *encrypt(char *str);
const char *decrypt(char *str1);

int main()
{
    char str[100],str1[100];

    //Encryption
    printf("Enter String for encryption\n");
    gets(str);
    encrypt(str);
    printf("%s after encryption is %s\n",str,encrypt(str));

    //Encryption
    printf("Enter String for decryption\n");
    gets(str1);
    decrypt(str1);
    printf("%s after decryption is %s",str1,decrypt(str1));
    return 0;
}

const char *encrypt(char *str)
{
    char en[100];
    int i=0;
    for(;i<100;i++)
    {
        en[i]=str[i]+1;
    }
    en[i]='\0';
    return en;
}

const char *decrypt(char *str1)
{
    char de[100];
    int i=0;
    for(;i<100;i++)
    {
        de[i]=str1[i]-3;
    }
    de[i]='\0';
    return de;
}

Answer 1

You are returning a pointer to automatic variables en and de which are stored in the stack. This in turn means after returning from the functions encrypt and decrypt their place in the memory can be used by any other variable.

so to correct that, you need to define en and de as static.

const char *encrypt(char *str){

    static char en[100];
    int i=0;
    for(;i<100;i++){
        en[i]=str[i]+1;
        }
    en[i]='\0';
    return en;
}

const char *decrypt(char *str1){
    static char de[100];
    int i=0;
    for(;i<100;i++){
        de[i]=str1[i]-3;
        }
    de[i]='\0';
    return de;
}

Though a more suitable and safer way to implement that would be:

#include<stdio.h>

void encrypt(char *str, char *encStr);
void decrypt(char *str1, char* decStr);

int main()
{
    char str[100], encDecStr[100];

    //Encryption
    printf("Enter String for encryption\n");
    scanf("%s", str);
    encrypt(str, encDecStr);
    printf("%s after encryption is %s\n",str,encDecStr);

    //Decryption
    printf("Enter String for decryption\n");
    scanf("%s", str);
    decrypt(str, encDecStr);
    printf("%s after decryption is %s",str,encDecStr);
    return 0;
}

void encrypt(char *str, char *encStr)
{
    for(char *c = str; *c != '\0'; c++)
    {
        *encStr++ = *c + 1;
    }
    *encStr='\0';
}

void decrypt(char *str1, char* decStr)
{
    for(char *c = str1; *c != '\0'; c++)
    {
        *decStr++ = *c - 1;
    }
    *decStr++='\0';
}

Note: The code was not fully tested for different use cases.

Answer 2

There are quite a number of errors in your code:

1. Returning arrays with local storage duration:
   The array's life time ends (i.e. it ceases to exist) as soon as you exit from the function, thus the pointer returned is dangling, reading from it is undefined behaviour
2. You write beyond the bounds of your local array:
   en[i] = '\0' with i being 100 after the loop is out of the range of valid indices from 0 to 99, which again is undefined behaviour.
3. You have differing offsets for encrypting (1) and decrypting (3).
4. Simply adding an offset without further checks (or modulo operations) will produce different character sets for input and output strings (might be your least problem...).
5. You always en-/decode the entire array, which is more than actually needed. Additionally the terminating null character then is encoded as well, resulting in different lengths of input and output and garbage at the end of encoded string.
6. Use of gets is dangerous as it allows a user to write beyond the input array, resulting in undefined behaviour. This is the reason why it actually has been removed from C language with C11 standard – which introduces a safe alternative gets_s. Yet another alternative (especially for pre-C11 code) is fgets.

For the dangling pointer problem there are several options:

1. Making the array static (as mentioned already):
   The disadvantage of this approach is that the function is not thread-safe any more. Additionally calling the function more than once overwrites previous results, if you haven't evaluated already or copied them they are lost.
2. Returning a dynamically allocated array, see malloc function.
   This comes with the risk of the caller forgetting to free the allocated memory again, which would result in a memory leak
3. Changing the input array in place:
   Disadvantage of is having to copy the input into a backup if it is yet needed afterwards.
4. Letting the caller provide the buffer.

Last option is most flexible and most idiomatic one, so I'll concentrate on this one:

void caesar(char const* input, char* output, int offset)
{
    int const NumChars = 'z' - 'a';
    offset = offset % NumChars + NumChars;
    //              ^ assures valid range, can still be negative
    //                         ^ assures positive value,  possibly
    //                           out of range, but will be fixed later
    for(;*input; ++input, ++output)
    {
        int c = *input - 'a';
        // distance from character a
        c = (c + offset) % NumChars;
        //               ^ fixes too large offset
        *output = 'a' + c; 
    }
    // as now iterating *until* null character found we now *need*
    // to add it (in contrast to original version with whole array!)
    *output = 0;
}

This variant includes an interesting idea: Let the caller define the offset to be applied! The modulo operation assures the valid range of the character, no matter how large the offset is. The great thing about: If a user encoded with some number n, exactly the same function can be used to decode again with -n (which is why I simply named it caesar according to the algorithm it implements). Note that this is untested code; if you find a bug please fix yourself...

Be aware, though, that this code still has two weaknesses:

1. It assumes ASCII encoding or compatible – at least one where letters a to z are in contiguous range, a is first character, z is last one. This is not true for all encodings, though, consider the (in-?)famous EBCDIC.
2. It assumes all input is in range of Latin minuscules only (from a - z), it does not consider white-space, digits, capital letters, punctuation marks... You might want to include special handling for all of these or at least the ones you might use.

You could fix these e.g. (many other variants are thinkable as well) by

1. converting all characters to either upper or lower case (toupper((unsigned char)*input) – assuming case doesn't matter)
2. search in an array of valid characters ("ABC...XYZ012...89") for the appropriate index and if found encode like above with NumChars being array length, otherwise (whitespace, punctuation) just leave as is.

In any case, the function would then be called like:

char input[128]; // prefer powers of 2 for array lengths...
char en[sizeof(input)];
char de[sizeof(input)];

gets_s(input, sizeof(input));
caesar(input, en, 7);
// en contains encrypted string, could e.g. be printed now
caesar(en, de, -7);
// de contains decrypted string

// you could even encode/decode in place:
caesar(input, input, 7);
// just be aware that this will fail, though, for the case if input
// and output overlap and input starts before output,  as then at
// some point already encoded values will be read as input again:
// caesar(input, input + 1, 7) fails!!!

Answer 3

Your code does not handle a special case for the character 'z'. Thus

en[i]=str[i]+1;

Causes the character '{' to be written to the array en instead. For learning more about why this happens, I recommend you look at ASCII tables and looking at the integer values for alphabets.

Secondly, did you mean to type -3 in there?

de[i]=str1[i]-3;

This won't work if you're planning on using the decrypt() function to decrypt strings that you made using encrypt() because you're adding 1 to the character while encrypting and then subtracting a different number when decrypting, so the result will appear garbled.

I rewrote your code for you, since this is a beginner program, I made as little changes as possible so you can understand it. I WOULD HIGHLY RECOMMEND NOT USING gets() though... See here.

#include<stdio.h>

const char *encrypt(char *str); 
const char *decrypt(char *str1);

int main()
{

    char str[100],str1[100];

    //Encryption
    printf("Enter String for encryption\n");
    gets(str);   // DON'T USE GETS!!! USE fgets(str, 100, stdin);
    encrypt(str);
    printf("%s after encryption is %s\n", str, encrypt(str));
    
    //Encryption
    printf("Enter String for decryption\n");
    gets(str1);  // DON'T USE GETS!!! USE fgets(str, 100, stdin);
    decrypt(str1);
    printf("%s after decryption is %s", str1, decrypt(str1));
    return 0;
    
    
}
const char *encrypt(char *str)
{

    char en[100];
    int i=0;
    for(; i<100; i++)
    {
        if (str[i] == 'z')
        {
             en[i] = 'a';
             continue;
        }
        en[i] = str[i] + 1;
    }
    en[i] = '\0';
    return en;
}


    
const char *decrypt(char *str1)
{
    char de[100];
    int i=0;
    for(; i<100; i++)
    {
        if (str[i] == 'a')
        {
             en[i] = 'z';
             continue;
        }
        de[i] = str1[i] - 1;
    }
    de[i] = '\0';
    return de;
}

Some criticisms

1. Like I said, gets() is really bad... See here for more details. Although it might be too complicated for you... A better alternative is fgets!

fgets(str, num, stdin)

takes user input from the console, and then stores it inside the array str, which must be large enough to store at least num characters. Don't worry about stdin if you don't know what that means. But be sure to always write it when using fgets as an alternative to gets

2. Like others have already posted, albeit using more technical jargon, it's a bad idea to declare an array inside a function and then return that array. You know the function ends when the return statement is hit, and at that point all the variables that were declared inside the function will get destroyed.

That doesn't necessarily mean that you can't read the data that was in them, but it becomes a probabilistic game where there's a teeny-tiny chance that the array will get corrupted after the function exits and before the data in that array is read. This is technically Undefined Behaviour.

I hope you know about pointers. You can modify the array which you passed as a parameter directly and then return that array, thus avoiding accessing an array outside it's lifetime.

Answer 4

There's some issues in your code :

1. Not a very big issue for a beginner , but you should avoid gets function. Because it doesn't check the input , it can cause buffers overflow and various security problems , try using fgets instead.

2. In encrypt , and decrypt functions , you are returning the address of an array located in the stack of the function , look :

    const char *encrypt(char *str){
        char en[100];
        int i=0;
        for(;i<100;i++){
             en[i]=str[i]+1;
        }
        en[i]='\0';
        return en;
    }
   

Here , Since the en array is declared inside the function , after the return you may get garbage string when trying to read it. The solution here , is to either malloc it , or declare a static array outside the function , and initialize it.

3. You are encrypting by adding 1 to the value of the string , and decrypt it by retrieving 3 . I don't know if this is what you intended to do.

Here's a new version of your code , try to check if it suits your need :

#include <stdio.h>
#include <stdlib.h>
#include <memory.h>
#include <string.h>

static char de[100] , en[100] ; 

const char *decrypt(char *str1){
    memset(de , 0 , 100) ; 
    int i=0;
    for(;i<strlen(str1);i++){
            de[i]=str1[i]-1;
     }                   
    de[i]='\0';
    return (const char*) de;
 }


const char* encrypt(char* str){ 
    memset(en , 0 , 100) ; 
    int i=0;
    for(;i<strlen(str);i++){
            en[i]=str[i]+1;
    }
    en[i]='\0';
    return (const char*) en;
}




int main(){
    char str[100],str1[100];

    //Encryption
    printf("Enter String for encryption\n");
    gets(str);
    encrypt(str);
    printf("%s after encryption is %s\n",str,encrypt(str));

    //Encryption
    printf("Enter String for decryption\n");
    gets(str1);
    decrypt(str1);
    printf("%s after decryption is %s",str1,decrypt(str1));
    return 0;
 }