Mail Archives: djgpp-workers/1999/07/25/04:01:33
On Sat, 10 Jul 1999, Mark E. wrote:
> Attached is a zip archive containing my current diffs against Readline
> 4.0. Let me know about any korrections, changes, etc.
I finally found the time to look at these patches. Here are some
minor comments to some of the patches:
1. complete.c:
+ #if defined (HAVE_PATHNAME_STYLE_DOS)
+ if (!temp && rl_filename_completion_desired
+ && (isalpha (pathname[0]) && pathname[1] == ':'))
+ temp = pathname + 1;
+ #endif
Use of `isalpha' when checking for the drive letter is not 100% safe:
DOS allows the 6 characters between upper-case Z and lower-case a to
be used as drive letters as well (since there can be a maximum of 32
block devices, but only 26 letters). This allowance is actually used
by Novell redirectors. So I suggest simply to lose `isalpha' in
fragments like the above. It is not really required here; but if you
want to play it safe, check the drive letter to be in the [A-z] range.
2.bind.c:
+ #if defined (HAVE_SETMODE) && defined(O_TEXT)
+ setmode (file, O_TEXT);
+ #endif
Is it safe to read .inputrc in text mode? Can we be sure that no
strange unprintable characters will ever be there, in particular in
the key-binding part?
One problem with text-mode reads is that it removes *all* CR
characters, not only those immediately preceding the LF. So, for
example, if somebody puts a bare CR character into their bindings,
they are in for a surprise...
For these reasons, I have chosen in my patches to use binary reads and
strip the CRs from CR-LF pairs manually. Perhaps I was too
paranoiac...
3. terminal.c:
- #if defined (__GO32__)
- screenwidth = ScreenCols ();
- screenheight = ScreenRows ();
- screenchars = screenwidth * screenheight;
- term_cr = "\r";
- term_im = term_ei = term_ic = term_IC = (char *)NULL;
- term_up = term_dc = term_DC = visible_bell = (char *)NULL;
-
- /* Does the __GO32__ have a meta key? I don't know. */
- term_has_meta = 0;
- term_mm = term_mo = (char *)NULL;
I'm probably missing something here--how is it supposed to work, after
you remove all this? The usual code uses termcap functions like
tgetnum etc., which we don't have.
- #if !defined (__GO32__)
if (term_backspace)
for (i = 0; i < count; i++)
tputs (term_backspace, 1, _rl_output_character_function);
else
- #endif /* !__GO32__ */
Same here: this will call tputs to backspace the cursor.
4. histfile.c:
+ #if defined (HAVE_SETMODE) && defined (O_TEXT)
+ setmode (file, O_TEXT);
+ #endif
Same comment as for .inputrc: are we sure that everything that goes to
the history file is printable text with no strange characters? If
not, using binary mode and manually stripping the CRs is safer.
! #if defined (HAVE_SETMODE) && defined (O_TEXT)
! setmode (file, O_TEXT);
! #endif
! write (file, buffer + i, chars_read - i);
Why do you need to do this before writing history? A file written in
binary mode will be read successfully in text mode, so this shouldn't
be necessary.
+ #if !defined (O_TEXT)
mode = overwrite ? O_WRONLY|O_CREAT|O_TRUNC|O_BINARY : O_WRONLY|O_APPEND|O_BINARY;
+ #else
+ mode = overwrite ? O_WRONLY|O_CREAT|O_TRUNC : O_WRONLY|O_APPEND;
+ mode |= O_TEXT;
+ #endif
output = history_filename (filename);
Same here.
I think `history_truncate_file' and `history_do_write' also need a
patch that handles the case of the initial dot in the history file
name. My patches included it.
5. display.c:
void
_rl_clear_screen ()
{
- #if !defined (__GO32__)
if (term_clrpag)
tputs (term_clrpag, 1, _rl_output_character_function);
else
- #endif /* !__GO32__ */
crlf ();
}
This and other patches in display.c seem to require termcap functions
which we don't have. See my comments under terminal.c above.
- Raw text -