C language: there is a trailing character after the last character of my output

Question

I am making a Caesar's Cipher for my lab sheet, and have made it able to encrypt 3 subtitution(Caesar's Cipher), which is the point of the exercise. But there has been one thing bugging me. First, there is a trailing character if i put it other than 3. For example, by typing "malware", and 2 for key. This is my code :

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

int main()
{
   char text[100];
   int key,i;

   printf("Please enter a word/sentence (lowercaps) for encrypting :\n ");
   fgets(text,100,stdin);
   printf("Please enter the key that you desire : eg:14\n");
   scanf("%d", &key);
   for(i=0;i<strlen(text);i++)
   {
      if (key>=26)
      {
         key=key%26;
      }
      if (text[i]==' ')
      {
         continue;
      }
      if(text[i]+key>'z')
      {
         text[i]-=97;
         text[i]+=26;
         text[i]+=key;
         text[i]%=26;
         text[i]+=97;
      }
      else
      {
         text[i]=text[i]+key;
      }
   }

   printf("this is your encrypted text : %s", text );
}

I hope I followed the correct indentation methods for coding. Got a lot of dislikes because of that

What do you mean exactly by trailing character ? The trailing whitespaces ? — Zermingore, Sep 24 '15 at 04:58
yes, special characters from ASCII I assume. But I'm not sure why did it appear. — Hafiz Syazwan, Sep 24 '15 at 05:01
It's a new line character that gets converted by your ciphering I think. — blakelead, Sep 24 '15 at 05:02
I don't think this is right `if (key>=26)`, Shouldn't it be `if (key>26)` . At the end you'll need to nul terminate you string you created in text. Before the final `printf` add `text[i] = '\0';` and then you may wish to print your final string with a newline at the end so change `text : %s` to `text : %s\n` — Michael Petch, Sep 24 '15 at 05:11
Of course you don't even need the `if (key>26)`. You can just take the straight modulo without the `if` statement and just replace it with `key=key%26;` — Michael Petch, Sep 24 '15 at 05:20
You should fix-up `key` once, before you start the loop. And you could use `key %= 26;` if you're sure the user entered a non-negative value. If they might have entered a negative value, maybe you need `key = (key % 26 + 26) % 26;`. Using `97` instead of `'97'` is not a particularly good idea. Your code doesn't deal with upper-case letters or punctuation very well. — Jonathan Leffler, Sep 24 '15 at 05:47
@JonathanLeffler I think a check for negative number would be better up front since Caesar cypher is only valid for positive numbers (and in the case of 0 it doesn't encrypt). — Michael Petch, Sep 24 '15 at 05:54
To be clear (I forgot to mention this, my bad) I browsed the Internet and decided to just go with just small letters, that's why I did not use toUpper — Hafiz Syazwan, Sep 24 '15 at 06:00
Oops; my comment about _`97` instead of `'97'`_ should read **`97` instead of `'a'`**; my apologies. — Jonathan Leffler, Sep 24 '15 at 06:01

chux - Reinstate Monica · Accepted Answer · 2015-09-24T05:53:13.913

2

Code is 1) not properly detecting when a char is a lower case letter 2) encrypting non-letters including '\n' from fgets() which is causing OP's "trailing character after the last character of my output".

Instead:

if (text[i] >= 'a' && text[i]<= 'z') {
   text[i] = (text[i] - 'a' + key)%26 + `a`;
}
else {
  ; // nothing 
}

Alternatively

if (islower((unsigned char) text[i]) {
   text[i] = (text[i] - 'a' + key)%26 + `a`;
}

Note: the above depends on char are encoded as ASCII.

A solution that does not depend on ASCII.

static const char lowercase[] = "abcdefghijklmnopqrstuvwxyz";
char *p = strchr(lowercase, text[i]);
if (p) {
  int offset = (p - lowercase + key)%26;
  text[i] = lowercase[offset];
}

edited Sep 24 '15 at 05:53

answered Sep 24 '15 at 05:46

chux - Reinstate Monica

143,097
13
135
256

how do I avoid the \n from getting into the buffer of fgets as well? Should I just go with your way? – Hafiz Syazwan Sep 24 '15 at 06:12
For "a solution that does not depend on ASCII" you might as well use `toUpper`. – Jongware Sep 24 '15 at 06:24
I have found quite an interesting solution to the trailing line, that is to use strcpn [function](http://stackoverflow.com/questions/2693776/removing-trailing-newline-character-from-fgets-input) . This is the [link](http://www.freebsd.org/cgi/man.cgi?query=strcspn&sektion=3) to the description of the function – Hafiz Syazwan Sep 24 '15 at 06:48
@Hafiz Syazwan to "... avoid the \n from getting into the buffer of fgets as well" --> do _not_ avoid it. Accept it. (BTW: it is not always there), _Then_ truncate it if exist. `text[strcspn(text, "/n")]=0;` is a safe, easy method to do that. – chux - Reinstate Monica Sep 24 '15 at 13:18
@Jongware It is unclear how using `toUpper()` (assume `toupper()`) would help. OP is apparently not using upper case characters. – chux - Reinstate Monica Sep 24 '15 at 13:21
@chux: I got confused - apparently OP only wants lower case (which he calls "lowercaps", a nice portmanteau) encrypted. No need for stealth when SHOUTING, methinks. – Jongware Sep 24 '15 at 16:20

Zermingore · Answer 2 · 2015-09-24T05:26:25.670

0

As Blake_Lead said, this '\0' character was changed in your cypher

Indeed I was wrong about the length of the buffer as fgets() puts a '\0'
From the manual page:

A terminating null byte ('\0') is stored after the last character in the buffer.

So, you just need to change your test

if (text[i]==' ')

by something like:

 if (text[i] < 'A' || text[i] > 'z' || (text[i] > 'Z' && text[i] < 'a') )

edited Sep 24 '15 at 05:26

answered Sep 24 '15 at 05:03

Zermingore

743
2
11
28

So , `text` is a string other places should be `null` . This cant be reason for such behaviour . – ameyCU Sep 24 '15 at 05:06

score 0 · Answer 3 · answered Sep 24 '15 at 05:21

0

I will simplify and correct this code to

#include <stdio.h>

int main() {
    char text[100];
    int key, i;
    printf("Enter a word / sentence (lowercaps) for encrypting : ");
    fgets(text, 100, stdin);
    printf("Enter the key that you desire (eg. 14) : ");
    scanf("%d", &key);
    key %= 26;    // Pull this out of the loop and remove the unnecessary if
    for (i = 0; text[i]; ++i) {    // Correct the loop condition
        if (text[i] == ' ') continue;
        if (text[i] + key > 'z')
            text[i] = (text[i] - 97 + 26) % 26 + 97;    // Simplify
        else
            text[i] += key;
    }
    printf("Encrypted text : %s\n", text);
    return 0;
}

Input

Enter a word / sentence (lowercaps) for encrypting : malware
Enter the key that you desire (eg. 14) : 2

Output

Encrypted text : ocnyctg

answered Sep 24 '15 at 05:21

Shreevardhan

12,233
3
36
50

Actually you should check for value if `key=2` and its code for `y` remains `y` does not change . – ameyCU Sep 24 '15 at 05:27
1

I keep looking at this code and thinking there is something wrong, you are encoding an extra character in `text` because of the newline returned by fgets – Michael Petch Sep 24 '15 at 05:45
1

Where does the key get added when `text[i] + key > 'z'`? Oops! This would be more obvious with a bigger key, such as 21, than with a small one like 3. – Jonathan Leffler Sep 24 '15 at 05:52

C language: there is a trailing character after the last character of my output

3 Answers3