MarkAlanFrank MarkAlanFrank - 22 days ago 12
C Question

Program not receiving required input

I have a problem with my homework. I have some code written by teacher and I suppose to edit it in order to make a calculator. So I added couple of lines i thought it will work, but sadly it's not the case. Program returns always that operands or operator is wrong. Can you have a look?

main.c

#include "stdio.h"
#include "evalexpression.h"

int main() {
char string[100];
int result;
result = InterCalc(string);
CalcFilter(result, string);
return 0;
}


evalexpression.c

#include "stdio.h"
#include "string.h"
#include "evalexpression.h"
#include "math.h"
#include "float.h"

static float f1, f2;
static char op;

int isValidExpression(const char *str) {
int res;
char ops[10];
res = sscanf(str, "%f %s %f", &f1, ops, &f2);
if (res == 3) {
if (ops[0] == '+' || ops[0] == '-' || ops[0] == '^' || ops[0] == '*' || ops[0] == '/') {
op = ops[0];
return 1;
} else
return 0;
} else
return 0;
}

int getOperator() {
return (op);
}

float getFstOperand() {
return (f1);
}

float getSecOperand() {
return (f2);
}

float getExprValue() {
int operation;
operation = getOperator();
switch (operation) {
case 1:
return (getFstOperand() + getSecOperand());
break;
case 2:
return (getFstOperand() - getSecOperand());
break;
case 3:
return (getFstOperand() / getSecOperand());
break;
case 4:
return (getFstOperand() * getSecOperand());
break;
case 5:
return (pow(getFstOperand(), getSecOperand()));
break;
default:
return 0;
}
}

int InterCalc(char *my_string) {
fgets(my_string, sizeof(my_string), stdin);
if (strcmp(my_string, "exit\n") == 0) {
printf("Program ended\n");
return 0;
} else
if (isValidExpression(my_string) == 0) {
printf("Expression error\n");
return 0;
} else
return 1;
}

void CalcFilter(int a, char *str) {
float calculation_value;
printf("Press 'E' to display the invalid line or press 'V' to display the valid line\n");
int choice;
choice = getchar();
switch (choice) {
case 'E':
case 'e':
if (a == 0) printf("The line %s is invalid.\n", str);
else if (a == 1) printf("There's nothing wrong with the line %s\n", str);
break;
case 'V':
case 'v':
if (a == 1) {
calculation_value = getExprValue();
printf("The result of %s is %f.\n", str, calculation_value);
}
if (a == 0) printf("The line %s is invalid\n", str);
break;
default:
printf("You haven't chosen the valid option of the switch\n");
break;
}
}

Answer

You should pass the size of the destination buffer to function InterCalc(). As written, it can only read sizeof(char*) - 1 bytes at a time. You should also check for end of file.

int InterCalc(char *my_string, size_t size) {
    if (fgets(my_string, size, stdin) == NULL
    ||  strcmp(my_string, "exit\n") == 0) {
        printf("Program ended\n");
        return 0;
    } else
    if (isValidExpression(my_string) == 0) {
        printf("Expression error\n");
        return 0;
    } else {
        return 1;
    }
}

Invoke from main():

#include <stdio.h>
#include "evalexpression.h"

int main(void) {
    char string[100];
    int result;
    result = InterCalc(string, sizeof(string));
    CalcFilter(result, string);
    return 0;
}

Notes:

  • you should use the <stdio.h> syntax for standard headers.

  • you should prevent buffer overflow by passing the maximum number of characters for %s formats in sscanf(): sscanf(str, "%f %9s %f", &f1, ops, &f2);

EDIT: There is another problem in GetExrValue(): you switch on values from 0 to 5 for op instead of the operation character. Here is a way to correct this:

float getExprValue(void) {
    switch (getOperator()) {
    case '+':
        return getFstOperand() + getSecOperand();
    case '-':
        return getFstOperand() - getSecOperand();
    case '/':
        return getFstOperand() / getSecOperand();
    case '*':
        return getFstOperand() * getSecOperand();
    case '^':
        return pow(getFstOperand(), getSecOperand());
    default:
        return 0;
    }
}
Comments