infinity cycle in screen initialization, when pdcurses app is used as pager
okbob opened this issue · 19 comments
I try to use pdcurses in application with redirected stdin ( cat file| myapp)
there is some pseudocode like
f_tty = fopen("/dev/tty", "r+")
newterm(termname(), stdout, f_tty);
This code works only when f_tty is same like stdin. In other cases It hangs in cycle
while( PDC_get_rows( ) < 1 && PDC_get_columns( ) < 1)
; /* wait for screen to be drawn and size determined */
PDC_get_rows returns always -1
Setting correct resize_term(size.ws_row, size.ws_col) doesn't help
backtrace is:
Breakpoint 2, PDC_get_rows () at ../vt/pdcgetsc.c:25
25 PDC_LOG(("PDC_get_rows() - called\n"));
(gdb) bt
#0 PDC_get_rows () at ../vt/pdcgetsc.c:25
#1 0x00007f46d7f34267 in PDC_scr_open () at ../vt/pdcscrn.c:322
#2 0x00007f46d7f216ad in initscr () at ../pdcurses/initscr.c:164
#3 0x00007f46d7f21f23 in newterm (type=0x7f46d7f43260 <_termname.2> "pdcurses", outfd=0x7f46d7f01780 <_IO_2_1_stdout_>,
infd=0x8183b0) at ../pdcurses/initscr.c:334
#4 0x0000000000406def in main (argc=<optimized out>, argv=<optimized out>) at src/pspg.c:2961
(gdb) c
But now, I look on newterm function, and all is clean now
SCREEN *newterm(const char *type, FILE *outfd, FILE *infd)
{
PDC_LOG(("newterm() - called\n"));
INTENTIONALLY_UNUSED_PARAMETER( type);
INTENTIONALLY_UNUSED_PARAMETER( outfd);
INTENTIONALLY_UNUSED_PARAMETER( infd);
return initscr() ? SP : NULL;
}
the all parameters are ignored :-/
Is there possibility how to use input from some specified stream instead just stdin?
Minimally the following code is partially wrong:
static void sigwinchHandler( int sig)
{
struct winsize ws;
INTENTIONALLY_UNUSED_PARAMETER( sig);
if (ioctl(STDIN_FILENO, TIOCGWINSZ, &ws) != -1)
if( PDC_rows != ws.ws_row || PDC_cols != ws.ws_col)
{
PDC_rows = ws.ws_row;
PDC_cols = ws.ws_col;
PDC_resize_occurred = TRUE;
if (SP)
SP->resized = TRUE;
}
}
...
There should be STDOUT_FILENO used, because for you, the dimensions are interesting for output.
No, there is not any check. Unfortunately, for using pspg for MSWin I need fully implemented newterm function. I cannot to redirect stdin inside the pager.
This is fundamental functionality for pager. On second hand, the complex application written primary for ncurses like pspg https://github.com/okbob/pspg is working with pdcurses very well with mouse support (and without bigger changes). Unfortunately, it cannot be used like pager now.
Um. An interesting point... I had not really considered the use for that redirection in PDCurses, which does not follow the terminfo model that ncurses uses.
At least for input, I think we can get this to work. The SCREEN structure would hold a new FILE *input_fp member (within the 'opaque' structure), which would default to NULL. When set to a non-NULL FILE pointer via newterm(), we would read characters from that file instead of doing platform-dependent checking for keyboard input.
It occurs to me that one might have redirected stdin by running, say,
./curses_program < /tmp/input_file.txt
Right now, that works with ncurses and with no flavor (except maybe DOS ones?) of PDCurses or PDCursesMod. The VT and framebuffer platforms, for example, assume input comes from a terminal rather than a file. So our check for reading a character should be
if( SP->opaque->input_fp)
fread() from SP->opaque->input_fd;
else if( !isatty( STDIN_FILENO))
fread() from STDIN;
else /* it's a for-real terminal */
read the character using the platform-dependent code;If that makes sense to you, I don't think it would be difficult to implement it, first for VT (which, I gather, is your current focus). We could then add it to other platforms.
We also have the question of what it means if you redirect the output. I don't have an answer for that one yet. The question has yet to arise. If it does, it will warrant discussion under a new issue.
In the meantime, I agree with @GitMensch that we should return ERR for non-NULL outfd parameters. (And an assert( NULL == outfd); would be good, with similar error-checking and/or assert()s for other terminfo function that are stubs in PDCursesMod.... such an assert() here would have told you right away that we weren't handling a non-NULL infd parameter, for example.)
If that makes sense to you, I don't think it would be difficult to implement it, first for VT (which, I gather, is your current focus). We could then add it to other platforms.
It makes at least sense to me ;-)
We also have the question of what it means if you redirect the output. I don't have an answer for that one yet.
Currently redirecting the output raises an error because col/lines is zero or the type does not match, see PDC_scr_open(). This has come up before "multiple times", related are also #15 and #50.
Currently redirecting the output raises an error because col/lines is zero or the type does not match
Yup, that's what I'm seeing. The situation is more complicated than my proposed remedy suggests.
For the initial issue raised by Pavel, we should have no serious problem. We will read from the actual terminal when we want (for example) to find out how many lines and columns the terminal has, and read from the file specified by infd otherwise.
Input file redirection complicates this, because we lose access to the terminal. We send a command to inquire about the screen size, and the terminal replies, but we don't "see" it because our input is coming from the redirected input. A bit of searching for how one gets a file descriptor to the "real" terminal hasn't turned up anything useful yet. I assume there's a way to do it, though, and will continue looking.
Interestingly, one can get output redirection to work by simply modifying the line
const int stdout_fd = 0;in the put_to_stdout() function in vt/pdcdisp.c so that the variable is equal to 2. This is actually the file descriptor for stderr. Do this, and run a command such as
./testcurs > /tmp/output.txt, and all is well : the PDCursesMod output goes to the (unredirected, still to the terminal) stderr, and if you have any printf() or similar calls in your program, they're appropriately re-directed to /tmp/output.txt.
Properly speaking, what we need is some sort of const int stdout_fd = get_terminal_output_file_descriptor();, i.e., the problem parallels that of getting the actual input file descriptor. At the very least, that function might be along the lines of
if( isatty( STDOUT_FILENO)) /* stdout is a tty; use stdout */
return( STDOUT_FILENO);
else if( isatty(STDERR_FILENO)) /* stdout isn't a tty, but stderr is; use stderr */
return( STDERR_FILENO);
else /* both are redirected, and we're out of options */
perror( "Both stdout and stderr are redirected");In Windows, we'd use
if( FILE_TYPE_CHAR == GetFileType( GetStdHandle( STD_OUTPUT_HANDLE)))to test for tty-ness. (I think. Just gave it a try under Wine, and it failed. But Wine's console support isn't perfect.)
changing STDIN to 2 - redirect to stderr - and to tty allows to start pspg in pager mode. But it crashes on assert
#5 0x00007fd54bd5d656 in __GI___assert_fail (assertion=0x7fd54bf35222 "(c[0] & 0xc0) == 0x80", file=0x7fd54bf351af "../vt/pdckbd.c", line=546,
function=0x7fd54bf35498 <__PRETTY_FUNCTION__.2> "PDC_get_key") at assert.c:101
#6 0x00007fd54bf2e04e in PDC_get_key () at ../vt/pdckbd.c:546
#7 0x00007fd54bf19dbf in wgetch (win=0xcc6190) at ../pdcurses/getch.c:541
Sorry, I've mixed issues here. The idea was to support redirected output by changing stdout_fd. (I've updated my previous comment to make some of that clearer.) I still don't know of any suitable trickery for handling redirected input.
The issue you initially raised (and which it sounds as if is still your focus) is what to do if newterm() is called, and the input stream is something other than stdin. In that case, we should still read from stdin during initwin(), because when we "read a character" at that point, we're getting information about the terminal (screen size, for example). Once initwin() has done its thing, we should ignore the keyboard and "read characters" from fdin.
To address a further issue you didn't raise (sorry!), I can't think of a use case for fdout != stdout, and therefore have no idea what the "right" way of handling it would be.
here is minimalistic very ugly hack patch. With this patch, the pspg can be used as pager. Unfortunately, it fully breaks keyboard - only basic ascii keys (pspg commands) works. I think so reason is missing settings for input stream (I opening stream just for any key):
[pavel@localhost vt]$ git diff
diff --git a/vt/pdckbd.c b/vt/pdckbd.c
index c9befcd9..0671d20d 100644
--- a/vt/pdckbd.c
+++ b/vt/pdckbd.c
@@ -38,7 +38,7 @@ static bool check_key( int *c)
{
bool rval;
#ifndef USE_CONIO
- const int STDIN = 0;
+ const int STDIN = 2;
struct timeval timeout;
fd_set rdset;
extern int PDC_n_ctrl_c;
@@ -65,7 +65,14 @@ static bool check_key( int *c)
{
rval = TRUE;
if( c)
- *c = getchar( );
+ {
+ FILE *f;
+
+ f = fdopen(dup(2), "r+");
+ *c = fgetc(f);
+ fclose(f);
+
+ }
}
else
rval = FALSE;
diff --git a/vt/pdcscrn.c b/vt/pdcscrn.c
index b6104f58..7061c608 100644
--- a/vt/pdcscrn.c
+++ b/vt/pdcscrn.c
@@ -100,7 +100,7 @@ static int set_win10_for_vt_codes( const bool setting_mode)
int PDC_rows = -1, PDC_cols = -1;
bool PDC_resize_occurred = FALSE;
-const int STDIN = 0;
+const int STDIN = 2;
chtype PDC_capabilities = 0;
static mmask_t _stored_trap_mbe;
@@ -212,7 +212,7 @@ static void sigwinchHandler( int sig)
struct winsize ws;
INTENTIONALLY_UNUSED_PARAMETER( sig);
- if (ioctl(STDIN_FILENO, TIOCGWINSZ, &ws) != -1)
+ if (ioctl(STDERR_FILENO, TIOCGWINSZ, &ws) != -1)
if( PDC_rows != ws.ws_row || PDC_cols != ws.ws_col)
{
PDC_rows = ws.ws_row;
So probably there are not any other dependencies, and reading from tty device should to work.
Thank you. You're right, it doesn't quite work, but I think it may be putting us on the right track. The idea of duplicating stderr and opening it for reading had not occurred to me... actually, I'd always thought of stderr (and stdout) as being things you wrote to, not read from.
I am able to run pspg against pdcurses for VT on Linux with attached patch, and it is works pretty well (all functionality, mouse, full keyboard).
diff --git a/pdcurses/initscr.c b/pdcurses/initscr.c
index 02c69c4f..df834a5a 100644
--- a/pdcurses/initscr.c
+++ b/pdcurses/initscr.c
@@ -148,6 +148,8 @@ MOUSE_STATUS Mouse_status;
extern RIPPEDOFFLINE linesripped[5];
extern char linesrippedoff;
+extern FILE *__infd;
+
WINDOW *initscr(void)
{
int i;
@@ -161,6 +163,9 @@ WINDOW *initscr(void)
if (!SP)
return NULL;
+ if (!__infd)
+ __infd = stdin;
+
if (PDC_scr_open() == ERR)
{
fprintf(stderr, "initscr(): Unable to create SP\n");
@@ -330,7 +335,7 @@ SCREEN *newterm(const char *type, FILE *outfd, FILE *infd)
INTENTIONALLY_UNUSED_PARAMETER( type);
INTENTIONALLY_UNUSED_PARAMETER( outfd);
- INTENTIONALLY_UNUSED_PARAMETER( infd);
+ __infd = infd;
return initscr() ? SP : NULL;
}
diff --git a/vt/pdckbd.c b/vt/pdckbd.c
index c9befcd9..5154274a 100644
--- a/vt/pdckbd.c
+++ b/vt/pdckbd.c
@@ -33,12 +33,12 @@ https://www.gnu.org/software/screen/manual/html_node/Control-Sequences.html
*/
extern bool PDC_resize_occurred;
+FILE *__infd = NULL;
static bool check_key( int *c)
{
bool rval;
#ifndef USE_CONIO
- const int STDIN = 0;
struct timeval timeout;
fd_set rdset;
extern int PDC_n_ctrl_c;
@@ -58,14 +58,14 @@ static bool check_key( int *c)
return( TRUE);
}
FD_ZERO( &rdset);
- FD_SET( STDIN, &rdset);
+ FD_SET( fileno(__infd), &rdset);
timeout.tv_sec = 0;
timeout.tv_usec = 0;
- if( select( STDIN + 1, &rdset, NULL, NULL, &timeout) > 0)
+ if( select( fileno(__infd) + 1, &rdset, NULL, NULL, &timeout) > 0)
{
rval = TRUE;
if( c)
- *c = getchar( );
+ *c = fgetc(__infd);
}
else
rval = FALSE;
diff --git a/vt/pdcscrn.c b/vt/pdcscrn.c
index b6104f58..d7ecbae7 100644
--- a/vt/pdcscrn.c
+++ b/vt/pdcscrn.c
@@ -8,6 +8,7 @@
#include <termios.h>
#include <sys/ioctl.h>
+extern FILE *__infd;
static struct termios orig_term;
#endif
@@ -100,7 +101,6 @@ static int set_win10_for_vt_codes( const bool setting_mode)
int PDC_rows = -1, PDC_cols = -1;
bool PDC_resize_occurred = FALSE;
-const int STDIN = 0;
chtype PDC_capabilities = 0;
static mmask_t _stored_trap_mbe;
@@ -111,14 +111,14 @@ void PDC_reset_prog_mode( void)
#ifdef USE_TERMIOS
struct termios term;
- tcgetattr( STDIN, &orig_term);
+ tcgetattr( fileno(__infd), &orig_term);
memcpy( &term, &orig_term, sizeof( term));
term.c_lflag &= ~(ICANON | ECHO);
term.c_iflag &= ~ICRNL;
term.c_cc[VSUSP] = _POSIX_VDISABLE; /* disable Ctrl-Z */
term.c_cc[VSTOP] = _POSIX_VDISABLE; /* disable Ctrl-S */
term.c_cc[VSTART] = _POSIX_VDISABLE; /* disable Ctrl-Q */
- tcsetattr( STDIN, TCSANOW, &term);
+ tcsetattr( fileno(__infd), TCSANOW, &term);
#endif
#ifndef _WIN32
if( !PDC_is_ansi)
@@ -190,7 +190,7 @@ void PDC_scr_close( void)
set_win10_for_vt_codes( FALSE);
#else
#if !defined( DOS)
- tcsetattr( STDIN, TCSANOW, &orig_term);
+ tcsetattr( fileno(__infd), TCSANOW, &orig_term);
#endif
#endif
PDC_doupdate( );
@@ -212,7 +212,7 @@ static void sigwinchHandler( int sig)
struct winsize ws;
INTENTIONALLY_UNUSED_PARAMETER( sig);
- if (ioctl(STDIN_FILENO, TIOCGWINSZ, &ws) != -1)
+ if (ioctl( fileno(__infd), TIOCGWINSZ, &ws) != -1)
if( PDC_rows != ws.ws_row || PDC_cols != ws.ws_col)
{
PDC_rows = ws.ws_row;
@@ -283,7 +283,7 @@ int PDC_scr_open(void)
if (!SP || PDC_init_palette( ))
return ERR;
- setbuf( stdin, NULL);
+ setbuf(__infd, NULL);
#ifdef USE_TERMIOS
sigemptyset(&sa.sa_mask);
sa.sa_flags = 0;
I am not able to test other platforms, and I am sure, so this patch is not well integrated to pdcurses well. Just it shows, so there is an possibility to work correctly, and necessity changes are not too big.
I tested it on VT because Linux and VT is my native platform, but I did this work as initial step for porting pspgf to mswin.
Finally got around to fixing (I'm fairly sure) your original issue : newterm( (ignored), stdout, non-stdin) will now work. Still need to pull your code in from your above comment to allow one to do 'traditional' command-line input redirection, though.
Also, the fix I made applies only to VT and Linux framebuffer/DRM at present; once we've confidence in it, it should be easy enough to extend it to other platforms.
Ah, hang on, I see what you mean... I was being quite dense here; it's nowhere near as simple as what I added.
A workaround you might consider : the WinGUI, SDL1/2, and X11 ports ought to work with paging programs without much trouble. They look for keyboard/mouse events via the Win32 API or SDLn or X11 libraries, rather than reading from stdin. So stdin redirection doesn't affect them.
For example, I patched testcurs.c with
--- a/demos/testcurs.c
+++ b/demos/testcurs.c
@@ -162,6 +162,13 @@ int main(int argc, char *argv[])
case 'z':
report_mouse_movement = TRUE;
break;
+ case 'i':
+ {
+ char buff[80];
+
+ if( fgets( buff, sizeof( buff), stdin))
+ printf( "Got '%s'\n", buff);
+ }
default:
break;and ran testcurs -i < junk.txt with WinGUI. The first line of junk.txt got read from stdin and was duly printed to the terminal. But testcurs got its input from the keyboard and mouse, unaffected by the redirection. Similarly for type junk.txt | testcurs -i. I tried the same experiment with SDL2 and X11 successfully.
WinCon says "Redirection is not supported" (this is a known issue, and causes grief in some situations.) I think the DOS port would work, since it reads keys and mouse via BIOS rather than stdin, but haven't tested it.
So for your original goal of getting pspg working on Windows, use of WinGUI actually ought to do the trick. However, I can imagine other situations where one might want to read from stdin while having PDCursesMod grab the keyboard and mouse input, and it'd be nice if it were possible in the VT and Linux framebuffer/DRM ports. (And very much so in WinCon.)
There is little bit unhappy written initialization. Almost all code is called from initscr. But when new_term is used, then it can be called before initscr or without initscr. Current state is wrong. The input stream is initialized (inside initscr is stdin used always), and in next step (too late) is set SP->opaque->input_fd from new_term. Storing output_fd, input_fd in opaque structure is not too happy, because you need to reset buffer in PDC_scr_open but opaque is initialized later when colour attributes are initialized.
With patch the pdcurses works with pspg well in both modes:
diff --git a/curspriv.h b/curspriv.h
index c8625e96..71015ca7 100644
--- a/curspriv.h
+++ b/curspriv.h
@@ -155,7 +155,9 @@ struct _opaque_screen_t
WINDOW **window_list;
unsigned trace_flags;
bool want_trace_fflush;
- FILE *output_fd, *input_fd;
};
+extern FILE *output_fd;
+extern FILE *input_fd;
+
#endif /* __CURSES_INTERNALS__ */
diff --git a/pdcurses/initscr.c b/pdcurses/initscr.c
index 054c844a..672b784c 100644
--- a/pdcurses/initscr.c
+++ b/pdcurses/initscr.c
@@ -143,6 +143,9 @@ int LINES = 0; /* current terminal height */
int COLS = 0; /* current terminal width */
int TABSIZE = 8;
+FILE *input_fd = NULL;
+FILE *output_fd = NULL;
+
MOUSE_STATUS Mouse_status;
extern RIPPEDOFFLINE linesripped[5];
@@ -161,6 +164,11 @@ WINDOW *initscr(void)
if (!SP)
return NULL;
+ if (!input_fd)
+ input_fd = stdin;
+ if (!output_fd)
+ output_fd = stdout;
+
if (PDC_scr_open() == ERR)
{
fprintf(stderr, "initscr(): Unable to create SP\n");
@@ -330,11 +338,11 @@ SCREEN *newterm(const char *type, FILE *outfd, FILE *infd)
PDC_LOG(("newterm() - called\n"));
INTENTIONALLY_UNUSED_PARAMETER( type);
+
+ /* These global variables should be set before initscr */
+ output_fd = outfd;
+ input_fd = infd;
win = initscr( );
- if( win && outfd != stdout)
- SP->opaque->output_fd = outfd;
- if( win && infd != stdin)
- SP->opaque->input_fd = infd;
return win ? SP : NULL;
}
diff --git a/vt/pdckbd.c b/vt/pdckbd.c
index 050e7974..0f5508cc 100644
--- a/vt/pdckbd.c
+++ b/vt/pdckbd.c
@@ -38,24 +38,12 @@ static bool check_key( int *c)
{
bool rval;
#ifndef USE_CONIO
- const int STDIN = 0;
struct timeval timeout;
fd_set rdset;
extern int PDC_n_ctrl_c;
if( PDC_resize_occurred)
return( TRUE);
- if( SP->opaque->input_fd)
- {
- rval = !feof( SP->opaque->input_fd);
- if( rval && c)
- {
- *c = fgetc( SP->opaque->input_fd);
- if( *c == EOF)
- rval = FALSE;
- }
- return( rval);
- }
#ifdef LINUX_FRAMEBUFFER_PORT
PDC_check_for_blinking( );
#endif
@@ -69,14 +57,14 @@ static bool check_key( int *c)
return( TRUE);
}
FD_ZERO( &rdset);
- FD_SET( STDIN, &rdset);
+ FD_SET( fileno( input_fd), &rdset);
timeout.tv_sec = 0;
timeout.tv_usec = 0;
- if( select( STDIN + 1, &rdset, NULL, NULL, &timeout) > 0)
+ if( select( fileno( input_fd) + 1, &rdset, NULL, NULL, &timeout) > 0)
{
rval = TRUE;
if( c)
- *c = getchar( );
+ *c = fgetc( input_fd);
}
else
rval = FALSE;
diff --git a/vt/pdcscrn.c b/vt/pdcscrn.c
index 380b125f..a0daa90e 100644
--- a/vt/pdcscrn.c
+++ b/vt/pdcscrn.c
@@ -100,7 +100,6 @@ static int set_win10_for_vt_codes( const bool setting_mode)
int PDC_rows = -1, PDC_cols = -1;
bool PDC_resize_occurred = FALSE;
-const int STDIN = 0;
chtype PDC_capabilities = 0;
static mmask_t _stored_trap_mbe;
@@ -111,14 +110,14 @@ void PDC_reset_prog_mode( void)
#ifdef USE_TERMIOS
struct termios term;
- tcgetattr( STDIN, &orig_term);
+ tcgetattr( fileno( input_fd), &orig_term);
memcpy( &term, &orig_term, sizeof( term));
term.c_lflag &= ~(ICANON | ECHO);
term.c_iflag &= ~ICRNL;
term.c_cc[VSUSP] = _POSIX_VDISABLE; /* disable Ctrl-Z */
term.c_cc[VSTOP] = _POSIX_VDISABLE; /* disable Ctrl-S */
term.c_cc[VSTART] = _POSIX_VDISABLE; /* disable Ctrl-Q */
- tcsetattr( STDIN, TCSANOW, &term);
+ tcsetattr( fileno( input_fd), TCSANOW, &term);
#endif
#ifndef _WIN32
if( !PDC_is_ansi)
@@ -190,7 +189,7 @@ void PDC_scr_close( void)
set_win10_for_vt_codes( FALSE);
#else
#if !defined( DOS)
- tcsetattr( STDIN, TCSANOW, &orig_term);
+ tcsetattr( fileno( input_fd), TCSANOW, &orig_term);
#endif
#endif
PDC_doupdate( );
@@ -286,7 +285,7 @@ int PDC_scr_open(void)
if (!SP || PDC_init_palette( ))
return ERR;
- setbuf( stdin, NULL);
+ setbuf( input_fd, NULL);
#ifdef USE_TERMIOS
sigemptyset(&sa.sa_mask);
sa.sa_flags = 0;