I tried to figure out why I always get a segmentation fault in my program which is part of an exercise, number 4-10, in The C Programming Language book of K&R.
I've implemented every function as it was teached by the book itself. I want to stick as much to it as possible.
Could anyone give me a hint? Would be very appreciated. Thanks!
/* An alternate organization uses getline to read an entire input line;
this makes getch and ungetch unnecessary. Revise the calculator to use
this approach. */
#include <ctype.h>
#include <stdio.h>
#include <stdlib.h> // for atof()
#define MAXLINE 100 // max size of input
#define MAXOP 100 // max size of operand or operator
#define MAXVAL 100 // maximum depth of val stack
#define NUMBER '0' // signal that a number was found
int getop(char[]);
int getl(char[], int);
void push(double);
double pop(void);
int li = 0; // input line index
char line[MAXLINE]; // one input line
int sp = 0; // next free stack position
double val[MAXVAL]; // value stack
/* reverse Polish calculator */
int main(void) {
int type;
double op2;
char s[MAXOP];
while ((type = getop(s)) != EOF) {
switch (type) {
case NUMBER:
push(atof(s));
break;
case ' ':
push(pop() pop());
break;
case '*':
push(pop() * pop());
break;
case '-':
op2 = pop();
push(pop() - op2);
break;
case '/':
op2 = pop();
if (op2 != 0.0)
push(pop() / op2);
else
printf("Error: Zero Divisor\n");
break;
case '\n':
printf("\t%.8g\n", pop());
break;
default:
printf("Error: Unknown Command %s\n", s);
break;
}
}
return 0;
}
/* getop: get next operator or numeric operand */
int getop(char s[]) {
int c, i;
if (line[li] == '\0') {
if (getl(line, MAXLINE) == 0)
return EOF;
else
li = 0;
}
while ((s[0] = c = line[li ]) == ' ' || c == '\t')
;
s[1] = '\0';
if (!isdigit(c) && c != '.') return c; // not a number
if (isdigit(c)) // collect integer part
while (isdigit(s[ i] = c = line[li ]))
;
if (c == '.')
while (isdigit(s[ i] = c = line[li ]))
;
s[i] = '\0';
li--;
return NUMBER;
}
/* getl: get line into s, return length */
int getl(char s[], int lim) {
int c, i;
i = 0;
while (--lim > 0 && (c = getchar()) != EOF && c != '\n')
s[i ] = c;
if (c == '\n')
s[i ] = c;
s[i] = '\0';
return i;
}
/* push: push f onto value stack */
void push(double f) {
if (sp < MAXVAL)
val[sp ] = f;
else
printf("Error: Stack full, can't push %g\n", f);
}
/* pop: pop and return top value from stack */
double pop(void) {
if (sp > 0)
return val[--sp];
else {
printf("Error: Stack empty\n");
return 0.0;
}
}
Thanks to MikeCAT the program is now fully working. Here the source code:
/* An alternate organization uses getline to read an entire input line;
this makes getch and ungetch unnecessary. Revise the calculator to use
this approach. */
#include <ctype.h>
#include <stdio.h>
#include <stdlib.h> // for atof()
#define MAXLINE 100 // max size of input
#define MAXOP 100 // max size of operand or operator
#define MAXVAL 100 // maximum depth of val stack
#define NUMBER '0' // signal that a number was found
int getop(char[]);
int getl(char[], int);
void push(double);
double pop(void);
int li = 0; // input line index
char line[MAXLINE]; // one input line
int sp = 0; // next free stack position
double val[MAXVAL]; // value stack
/* reverse Polish calculator */
int main(void) {
int type;
double op2;
char s[MAXOP];
while ((type = getop(s)) != EOF) {
switch (type) {
case NUMBER:
push(atof(s));
break;
case ' ':
push(pop() pop());
break;
case '*':
push(pop() * pop());
break;
case '-':
op2 = pop();
push(pop() - op2);
break;
case '/':
op2 = pop();
if (op2 != 0.0)
push(pop() / op2);
else
printf("Error: Zero Divisor\n");
break;
case '\n':
printf("\t%.8g\n", pop());
break;
default:
printf("Error: Unknown Command %s\n", s);
break;
}
}
return 0;
}
/* getop: get next operator or numeric operand */
int getop(char s[]) {
int c, i;
if (line[li] == '\0') {
if (getl(line, MAXLINE) == 0)
return EOF;
else
li = 0;
}
while ((s[0] = c = line[li ]) == ' ' || c == '\t')
;
s[1] = '\0';
if (!isdigit(c) && c != '.') return c; // not a number
i = 0;
if (isdigit(c)) // collect integer part
while (isdigit(s[ i] = c = line[li ]))
;
if (c == '.')
while (isdigit(s[ i] = c = line[li ]))
;
s[i] = '\0';
li--;
return NUMBER;
}
/* getl: get line into s, return length */
int getl(char s[], int lim) {
int c, i;
i = 0;
while (--lim > 0 && (c = getchar()) != EOF && c != '\n')
s[i ] = c;
if (c == '\n')
s[i ] = c;
s[i] = '\0';
return i;
}
/* push: push f onto value stack */
void push(double f) {
if (sp < MAXVAL)
val[sp ] = f;
else
printf("Error: Stack full, can't push %g\n", f);
}
/* pop: pop and return top value from stack */
double pop(void) {
if (sp > 0)
return val[--sp];
else {
printf("Error: Stack empty\n");
return 0.0;
}
}
CodePudding user response：
You used an indeterminate value of uninitialized non-static local variable i at the line
while (isdigit(s[ i] = c = line[li ]))
of the function getop. You have to initialize i before using its value.
You should learn to use a debugger to determine at which line your program failed and the value of which variables is not expected. For example, I used GDB in this way:
D:\Temp>gcc -Wall -Wextra -g3 -o a a.c
D:\Temp>gdb a
GNU gdb (GDB) 7.6.1
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3 : GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "mingw32".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from D:\Temp\a.exe...done.
(gdb) r
Starting program: D:\Temp/a.exe
[New Thread 9144.0x6d24]
[New Thread 9144.0x2f1c]
22 22-/
Program received signal SIGSEGV, Segmentation fault.
0x004015fe in getop (s=0x61feac "2") at a.c:79
79 while (isdigit(s[ i] = c = line[li ]))
(gdb) p s
$1 = 0x61feac "2"
(gdb) p line
$2 = "22 22-/\n", '\000' <repeats 91 times>
(gdb) p i
$3 = 14027921
(gdb) p li
$4 = 2
(gdb)