set *errin* isTTY properly in package mode (-p)

[RussWoodroofe]

When running in package mode (-p), as used by xgap, GAP
sets errin to be the same as stdin. It should at the same
time also set the isTTY property of errin to be the same as
that for stdin.

[codecov]

Codecov Report

Merging #2269 into master will increase coverage by 0.27%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2269      +/-   ##
==========================================
+ Coverage   70.57%   70.85%   +0.27%     
==========================================
  Files         481      480       -1     
  Lines      253237   253236       -1     
==========================================
+ Hits       178728   179435     +707     
+ Misses      74509    73801     -708

Impacted Files	Coverage Δ
src/system.c	65.87% <0%> (-0.13%)	⬇️
src/hpc/traverse.c	95.45% <0%> (-0.03%)	⬇️
src/gap.h
src/sysfiles.c	36.87% <0%> (+0.07%)	⬆️
src/lists.c	66.47% <0%> (+0.11%)	⬆️
src/hpc/threadapi.c	36.9% <0%> (+0.18%)	⬆️
src/funcs.c	79.37% <0%> (+1.09%)	⬆️
src/stats.c	86.92% <0%> (+1.34%)	⬆️
hpcgap/lib/package.gi	40% <0%> (+3.38%)	⬆️
src/exprs.c	93.27% <0%> (+3.62%)	⬆️
... and 3 more

[fingolfin]

This code is not "obviously" right. If we really "know" that syBuf[0] refers to stdin/stdout, i'd rather copy all three values from it.

Or else, use isatty directly, i.e. syBuf[2].isTTY = isatty(syBuf[2].fp);.

[RussWoodroofe]

I agree with your comment. To expand a little, the line added and preceding line should read either as:

syBuf[2].fp = syBuf[0].fp;  syBuf[2].echo = syBuf[0].echo;
syBuf[2].isTTY = syBuf[0].isTTY;

or else as:

syBuf[2].fp = fileno(stdin);  syBuf[2].echo = fileno(stdout);
syBuf[2].isTTY = (isatty(syBuf[2].fp) && ttyname(syBuf[2].fp) != NULL);

The former one seems a little more correct to me, but the latter is a bit more conservative. In the former case, the next line in the code (setting syBuf[3].fp) should probably also be changed to a similar style. I'm not sure if it should also set syBuf[3].echo or not. The code at the top of the init routine is a little inconsistent in that respect.

Note that the ttyname check in the latter solution mirrors the check for isTTY earlier in the code.

I'll try and edit the proposed patch for the less conservative solution.

[fingolfin]

Either of the two solutions would be fine by me (assuming they both "work" for you, i.e. solve your original problem).

[RussWoodroofe]

I actually tested both (including sending an ACK in a break loop), and both seemed to work. Neither seems likely to cause any new problems.

I edited the pull request to match the first solution. (Let me know if I did it incorrectly.)

[fingolfin]

LGTM; but when merging, I'd squash thos into a single commit