On Tue, Feb 21, 2017 at 07:19:13AM +0100, Otto Moerbeek wrote:
> On Tue, Feb 21, 2017 at 07:00:34AM +0100, Otto Moerbeek wrote:
>
> > On Tue, Feb 21, 2017 at 04:08:57AM +0100, Martijn Dekker wrote:
> >
> > > Upon encountering a parsing error, bc(1) passes an error message on to
> > > dc(1), which writes the error message to standard output along with the
> > > normal output.
> > >
> > > That is a bug. Error messages should go to standard error instead, as
> > > POSIX specifies:
> > > http://pubs.opengroup.org/onlinepubs/9699919799/utilities/bc.html#tag_20_09_10
> > >
> > > GNU 'bc' and Solaris 'bc' act like POSIX says and write error messages
> > > to standard error.
> > >
> > > Bizarrely, the exit status of bc(1) is left unspecified:
> > > http://pubs.opengroup.org/onlinepubs/9699919799/utilities/bc.html#tag_20_09_18
> > > And indeed, all versions of 'bc' exit with status 0 if there is an input
> > > error such as a parsing error, so the exit status cannot be used to
> > > catch it. That leaves examining standard error as the only method for a
> > > program calling bc(1), such as a shell script, to distinguish between an
> > > error state and normal operation. That is, with this bug, there is no
> > > way at all.
> > >
> > > The following example shell function transparently hardens bc(1) by
> > > intercepting standard error and exiting the program or subshell if an
> > > error was produced.
> > >
> > > bc() {
> > > _bc_err=$(command -p bc "$@" 1>&3 2>&1)
> > > [ -z "${_bc_err}" ] && return
> > > printf '%s\n' "$0: bc(1) caught errors:" "${_bc_err}" 1>&2
> > > exit 125
> > > } 3>&1
> > >
> > > The patch below fixes bc(1) so error messages are written directly to
> > > standard error and the above shell function works as expected. As a side
> > > effect, yyerror() is simplified.
> > >
> > > Another side effect is that bc(1) error messages are no longer neatly
> > > included in the generated dc(1) source when debugging it using 'bc -c'.
> > > But I don't think that is actually a problem; they are just printed to
> > > standard error instead. In fact, the patch makes 'bc -c' act like
> > > Solaris. If others find this problematic, the patch could be extended to
> > > restore the old behaviour only if '-c' is given.
> > >
> > > The manual page does not document error message behaviour one way or
> > > another. Since the patch implements standard behaviour, no change seems
> > > necessary there.
> > >
> > > Thanks,
> > >
> > > - M.
> > >
> >
> > Thanks for the diff. I am now wondering why I wrote it this way....
> > Likely beacuse the original bc had a similar approach.
> > Anyway, I'll try to look at this the coming days,
>
> Indeed, 4.4BSD bc does this:
>
> yyerror( s ) char *s; {
> if(ifile > sargc)ss="teletype";
> printf("c[%s on line %d, %s]pc\n", s ,ln+1,ss);
> fflush(stdout);
> cp = cary;
> crs = rcrs;
> bindx = 0;
> lev = 0;
> b_sp_nxt = &b_space[0];
> }
>
> My original goal was to make a bc that produced the same dc commands
> as the reference implementation I used.
>
> You can now see that your diff skips the 'c' commands an in that
> changes behaviour. Pondering if introducing a way to write to
> stderr in dc(1) would be better...
>
> -Otto
This should fix the problem and keep the code more close to the
reference implementation. Also, I noticed an omission in the
determination if we're interactive or not, so I fixed that as well.
-Otto
Index: dc/bcode.c
===================================================================
RCS file: /cvs/src/usr.bin/dc/bcode.c,v
retrieving revision 1.49
diff -u -p -r1.49 bcode.c
--- dc/bcode.c 27 Mar 2016 15:55:13 -0000 1.49
+++ dc/bcode.c 21 Feb 2017 10:25:41 -0000
@@ -67,6 +67,7 @@ static __inline struct number *pop_numbe
static __inline char *pop_string(void);
static __inline void clear_stack(void);
static __inline void print_tos(void);
+static void print_err(void);
static void pop_print(void);
static void pop_printn(void);
static __inline void print_stack(void);
@@ -195,6 +196,7 @@ static const struct jump_entry jump_tabl
{ 'a', to_ascii },
{ 'c', clear_stack },
{ 'd', dup },
+ { 'e', print_err },
{ 'f', print_stack },
{ 'i', set_ibase },
{ 'k', set_scale },
@@ -491,6 +493,18 @@ print_tos(void)
if (value != NULL) {
print_value(stdout, value, "", bmachine.obase);
(void)putchar('\n');
+ }
+ else
+ warnx("stack empty");
+}
+
+static void
+print_err(void)
+{
+ struct value *value = tos();
+ if (value != NULL) {
+ print_value(stderr, value, "", bmachine.obase);
+ (void)putc('\n', stderr);
}
else
warnx("stack empty");
Index: dc/dc.1
===================================================================
RCS file: /cvs/src/usr.bin/dc/dc.1,v
retrieving revision 1.29
diff -u -p -r1.29 dc.1
--- dc/dc.1 10 Dec 2016 21:13:25 -0000 1.29
+++ dc/dc.1 21 Feb 2017 10:25:41 -0000
@@ -185,6 +185,10 @@ operator is a non-portable extension.
All values on the stack are popped.
.It Ic d
The top value on the stack is duplicated.
+.It Ic e
+Equivalent to
+.Ic p ,
+except that the output is written to the standard error stream.
.It Ic f
All values on the stack are printed, separated by newlines.
.It Ic G
Index: bc/bc.y
===================================================================
RCS file: /cvs/src/usr.bin/bc/bc.y,v
retrieving revision 1.49
diff -u -p -r1.49 bc.y
--- bc/bc.y 23 Nov 2015 09:58:55 -0000 1.49
+++ bc/bc.y 21 Feb 2017 10:25:41 -0000
@@ -969,7 +969,7 @@ yyerror(char *s)
putchar('\\');
putchar(*p);
}
- fputs("]pc\n", stdout);
+ fputs("]ec\n", stdout);
free(str);
}
@@ -1135,7 +1135,8 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
- interactive = isatty(STDIN_FILENO);
+ interactive = isatty(STDIN_FILENO) && isatty(STDOUT_FILENO) &&
+ isatty(STDERR_FILENO);
for (i = 0; i < argc; i++)
sargv[sargc++] = argv[i];
