Let's Fix It
The minimal changes needed to get it to work are:
- You forgot to initialize the
one counter.
- You should be counting up, not down (i.e.
one++ not one--).
- You are not adding the parity bit to the output, but just duplicating the input.
- You need to add a terminating newline (
\n) in the printf of the output.
With fixes, it is:
#include<stdio.h>
#include<stdlib.h>
#include<string.h>
int main (){
char msg_bit_input[128];
char msg_bit_output[128];
char caruno='1';
int i;
int one = 0;
printf("Insert the byte: ");
scanf("%s", msg_bit_input);
strcpy (msg_bit_output, msg_bit_input);
for(i=0;i<8;i++){
if(msg_bit_input[i]==caruno){
one++;
}
}
if(one%2==0){
//byte is even
strcat (msg_bit_output, "0");
} else if(one%2==1){
//byte is uneven
strcat (msg_bit_output, "1");
}
printf("The result is: %s", msg_bit_output);
return (EXIT_SUCCESS);
}
Try it out online: https://onlinegdb.com/Bk30ef1s8.
Let's Simplify and Improve
But the program is unnecessarily complicated. First, it's not needed to use a separate output and input: you're taking input as-is and adding a parity bit to it. We simplify to:
//...
int main (){
char bits[128];
char caruno='1';
int i;
int one = 0;
printf("Insert the byte: ");
scanf("%s", bits);
for(i=0;i<8;i++){
if(bits[i]==caruno){
one++;
}
}
if(one%2==0){
//byte is even
strcat (bits, "0");
} else if(one%2==1){
//byte is uneven
strcat (bits, "1");
}
printf("The result is: %s", bits);
return (EXIT_SUCCESS);
}
Try it: https://onlinegdb.com/S1UwWMksU
Then, we note that the test for even/odd parity can only have two outcomes - they are alternatives, so if the result is not even, we know it must be odd, so no need to check that:
//...
if (one%2==0)
//byte is even
strcat (bits, "0");
else
//byte is odd
strcat (bits, "1");
//...
Try it: https://onlinegdb.com/Skrc-Gki8
The use of the caruno variable is unnecessary: it's hardly a "magic constant", we quite know what it means!
int main (){
char bits[128];
int i;
int one = 0;
printf("Insert the byte: ");
scanf("%s", bits);
for(i=0;i<8;i++){
if(bits[i]=='1'){
one++;
}
}
//...
Try it: https://onlinegdb.com/ByAA-MkjU
We then notice that scanf has no way of knowing how long is the buffer it receives input into. If the user provides longer input, scanf will happily overflow that buffer and scribble all over the stack. We'd want the program to be robust in face of bad inputs - that's just a change to the scanf format argument, to provide the width. There's a caveat: scanf will always append the zero terminating character to the input, so we need to provide the buffer size less 1 to leave room for that zero:
scanf("%127s", bits);
Try it: https://onlinegdb.com/SJJrqMyj8
And now we also notice that we don't really test whether there's room in the input string to append the output. Instead of appending the output, we can print it separately:
int main (){
char bits[128];
int i;
int one = 0;
char result;
printf("Insert the byte: ");
scanf("%127s", bits);
for(i=0;i<8;i++){
if(bits[i]=='1'){
one++;
}
}
if (one%2==0)
result = '0'; //byte is even
else
result = '1'; //byte is odd
printf("The result is: %s%c", bits, result);
return (EXIT_SUCCESS);
}
Try it: https://onlinegdb.com/Bk0w9M1iU
One could argue that the limitation of input to just 8 bits seems unnecessary - you never check for it. If it were necessary, we could check for it explicitly.
Let's say we don't care whether the input is exactly 8 bits or not: we'll return whatever bits were entered, and append a parity bit. We do this by looping not over i, but using a pointer to the string buffer, and comparing the character to '\0' - that's the null terminator where we should break the loop. Note that the iterator (the bufp pointer) can point to const char, since we don't intend the loop to be modifying the characters.
At that point we can also detect invalid input that contains characters other than 0 or 1. We add the error function to print the error output. We can also get rid of the string.h header inclusion, since we don't use any string operations from that header:
#include <stdio.h>
#include <stdlib.h>
int error(const char *message)
{
fprintf(stderr, "\nError: %s.\n", message);
return EXIT_FAILURE;
}
int main (){
const char bits[128];
char *bitp;
int one = 0;
char result;
printf("Enter one or more binary bits: ");
scanf("%127s", bits);
for (bitp = bits; *bitp != '\0'; ++bitp)
{
if (*bitp == '1')
++one;
else if (*bitp != '0')
return error("invalid input");
}
if (one%2==0)
result = '0'; //byte is even
else
result = '1'; //byte is odd
printf("The result is: %s%c", bits, result);
return (EXIT_SUCCESS);
}
Try it: https://onlinegdb.com/B1Ux-XyiI
We should now test if we handle another input error condition - if there's a premature end of input stream. This can be triggered on windows by pressing Ctrl-Z then Enter (this closes the input stream to the console process), or by pressing Ctrl-D on Unix.
In onlinegdb, you can trigger this error condition by pressing ^D in the console window after you Run the program:
And Finally...
Finally, it would be a good idea to leverage more modern C features: you should declare and initialize the variables close to the point of use, so that they are easier to keep track of. The result variable can be initialized using the ternary operator expression, of the form: condition ? value_if_true : value_if_false. This entire expression becomes value_if_true if condition evaluated to true, or value_if_false if the condition evaluated to false. This helps with initialization at the point of declaration:
#include <stdio.h>
#include <stdlib.h>
int error(const char *message)
{
fprintf(stderr, "\nError: %s.\n", message);
return EXIT_FAILURE;
}
int main()
{
char bits[128];
printf("Enter one or more binary bits: ");
scanf("%127s", bits);
int ones = 0;
for (const char *bitp = bits; *bitp != '\0'; ++bitp)
{
if (*bitp == '1')
++ones;
else if (*bitp != '0')
return error("invalid input");
}
char result = (ones%2==0) ? '0' /*even*/ : '1' /*odd*/;
printf("The result is: %s%c", bits, result);
return EXIT_SUCCESS;
}
Try it: https://onlinegdb.com/SJ1DbmJi8
And Finally, Finally :)
One thing I don't particularly like is the bug-prone repetition of buf length in both the variable declaration and in scanf's format string.
Let's try the suggestion from this answer, and factor out the string input into a separate function:
#include <stdio.h>
#include <stdlib.h>
int error(const char *message)
{
fprintf(stderr, "\nError: %s.\n", message);
return EXIT_FAILURE;
}
void scan_string(char *buffer, unsigned length) {
char format[12]; // Support max int value for the format %<num>s
snprintf(format, sizeof(format), "%%%ds", length - 1); // Generate format
scanf(format, buffer);
}
int main()
{
char bits[128];
printf("Enter one or more binary bits: ");
scan_string(bits, sizeof(bits));
int ones = 0;
for (const char *bitp = bits; *bitp != '\0'; ++bitp)
{
if (*bitp == '1')
++ones;
else if (*bitp != '0')
return error("invalid input");
}
char result = (ones%2==0) ? '0' /*even*/ : '1' /*odd*/;
printf("The result is: %s%c", bits, result);
return EXIT_SUCCESS;
}
Try it: https://onlinegdb.com/HyEsbQJoI
