C Caesar Cipher Function Call Not Behaving as Expected

Question

I am trying to build a program that will do a simple caesar cipher on a text file with single strings with no spaces on each line. For some reason, my cipher function is not shifting text and I am cutting off the strings at various lengths of characters. Can you see where I am messing up with my function call in the while loop?

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

#define FILE_NAME "./infile.txt"

void caeser (char * ch, int shift)
{
    int i = 0;
    int len = strlen(ch);

    while (ch[i] < len)
    {
        if (islower(ch[i]))
            ch[i] = ((ch[i] - 'a' + shift) % 26 + 'a');
        else
            ch[i] = ((ch[i] - 'A' + shift) % 26 + 'A');
    }i++;

    printf("Caesar Cipher = %s\n", ch);

}

int main(void)
{
    char *  c = malloc( sizeof(char) * 1000);


    FILE* fp = fopen (FILE_NAME, "r");
    if (fp == NULL)
    {
        printf("Can't open %s\n", FILE_NAME);
        exit(EXIT_FAILURE);
    }

    while(fgets(c, sizeof(c), fp) != 0)
    {
        printf("%s\n", c);
        caeser(c, 1);
    }

    fclose(fp);
    fp = NULL;
    return 0;
}

Ouch! The `fgets(c, sizeof(c), stdin)` thing works only if `c` is a local array defined like so: `char c[1000]`. In your case, where you allocate memory, you must specify the allocated length: `fgets(c, 1000, stdin)`. (That's because `sizeof(c)` is the size of a pointer, usually 4 or 8.) — M Oehm, Nov 11 '18 at 18:24
The while loop condition seems... odd. Also, ` }i++;` is outside the while loop. — kfx, Nov 11 '18 at 18:24
This resource may be helpful: https://ericlippert.com/2014/03/05/how-to-debug-small-programs/ — kfx, Nov 11 '18 at 18:25

score 0 · Answer 1 · answered Nov 11 '18 at 19:50

I made a few changes in your code and I marked them in bold.

Array start from 0 and the end is the n-1 character.

you checked if ch[i] < len. ch[i] is a character in the i place of an array, not a number.

in each iteration, you need to increment i by 1. so you can get the next character.

for better design try to sand a printable format to the main function instead print in the function. you should return a pointer to string and print it in main.

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

#define FILE_NAME "./infile.txt"

void caeser (char * ch, int shift)
{
    int i = 0;
    int len = strlen(ch);

    while (**i < len-1**)
    {
        if (islower(ch[i]))
        {
            ch[i] = ((ch[i] - 'a' + shift) % 26 + 'a');
            **i++;**
        }
        else
        {
            ch[i] = ((ch[i] - 'A' + shift) % 26 + 'A');
            **i++;**
        }
    }
    printf("Caesar Cipher = %s\n", ch);
}

int main(void)
{
    char *  c = malloc( sizeof(char) * 1000);


    FILE* fp = fopen (FILE_NAME, "r");
    if (fp == NULL)
    {
        printf("Can't open %s\n", FILE_NAME);
        exit(EXIT_FAILURE);
    }

    while(fgets(c, sizeof(c), fp) != 0)
    {
        printf("%s\n", c);
        caeser(c, 1);
    }

    fclose(fp);
    fp = NULL;
    return 0;
}

`*` means something in C so it is not a good idea to try to mark lines as you did. Instead, post code that can be compiled without modification, and you can either describe the changes afterwards, or use code comments — M.M, Nov 12 '18 at 05:43
`while(fgets(c, sizeof(c), fp) != 0)` is incorrect. `c` is a pointer, `sizeof(c)` is the size of a pointer — M.M, Nov 12 '18 at 05:44

C Caesar Cipher Function Call Not Behaving as Expected

1 Answers1