C: While loop exit condition, using fgets for input

Question

I am working on a program that will convert a binary to a decimal, the user inputs the original number or presses q to quit. My while loop does not exit, the q is being interpreted as a decimal number. What is the best way to stop this and work around this issue? Below is expected output, actual output and my code.

Expected output:

Enter binary byte or press q to quit: 111
7

Enter binary byte or press q to quit: q
Goodbye!

Actual outout:

Enter binary byte or press q to quit: 111
7

Enter binary byte or press q to quit: q
65

Code:

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

#define STOP "q"

int convertbinary(char *binarynumber);


int main()
{
    int binaryreturn; 
    char *bnumber;
    int len; 
    bnumber = (char *) malloc(10 * sizeof(char *));

    printf("Enter binary byte or type q to quit:?\n");

    while (fgets(bnumber, 10, stdin) != STOP)
    {
        len = strlen(bnumber);
        if (bnumber[len - 1] == '\n')
        {
            bnumber[len - 1] = 0;
        }
        else
        {
            //blank line
        }

        binaryreturn = convertbinary(bnumber);
        printf("%d\n", binaryreturn); 
        printf("Enter binary byte or type q to quit:?\n");
    }


    free(bnumber);

    return 0; 
}

int convertbinary(char *binarynumber)
{
    int val = 0;

    while (*binarynumber != '\0')
        val = 2 * val + (*binarynumber++ - '0');

    return val;
}

Answer 1

As others have said, you need to compare strings with strcmp, instead of !=, as this is incorrect in C.

I also suggest checking the return value of the void* pointer from malloc, as this is good practice to do when allocating space for a dynamic array on the heap.

You also don't need to cast the return value from malloc() .

You can simply write:

char *bnumber = malloc(10 * sizeof *bnumber);

instead of:

char *bnumber = (char *) malloc(10 * sizeof(char *)); //Note

I also saw that you included math.h as one of your headers. There is no need for that header because you aren't using any of it's functions.

I also added some extra error checking in this example, as it will help prevent bugs in the program.

here is the code:

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

#define BUFFSIZE 10

int convertbinary(char *binarynumber);

int
main(void) {
    int binaryreturn;
    char *bnumber;
    size_t currsize = BUFFSIZE, len;
    const char *quit = "q";

    bnumber = malloc(currsize * sizeof *bnumber);
    if (!bnumber) {
        printf("Cannot allocate memory!\n");
        exit(EXIT_FAILURE);
    }

    printf("Enter binary byte or type q to quit: ");
    while (fgets(bnumber, currsize, stdin) != NULL) {
        len = strlen(bnumber);

        if (len > 0) {
            if (bnumber[len-1] == '\n') {
                bnumber[len-1] = '\0';
            }  else {
                printf("Buffer Exceeds length of %d\n", BUFFSIZE);
                exit(EXIT_FAILURE);
            }
        }

        if (!*bnumber) {
            printf("No number entered.\n");
            printf("\nEnter binary byte or type q to quit: ");
        } else {
            if (strcmp(bnumber, quit) == 0) {
                printf("Goodbye\n");
                break;
            }

            binaryreturn = convertbinary(bnumber);
            printf("binary number = %d\n", binaryreturn);
            printf("\nEnter binary byte or type q to quit: ");
        }
    }       

    free(bnumber);
    bnumber = NULL;

    return 0;
}

int 
convertbinary(char *binarynumber) {
    int val = 0;

    while (*binarynumber != '\0') {
        val = 2 * val + (*binarynumber++ - '0');
    }

    return val;
}