Looks good, ok nicm
On Tue, Feb 26, 2019 at 02:39:14AM +0100, Ingo Schwarze wrote:
> Hi Todd,
>
> Todd C. Miller wrote on Mon, Feb 25, 2019 at 01:06:02PM -0700:
> > On Mon, 25 Feb 2019 19:43:36 +0100, Ingo Schwarze wrote:
> >> Todd C. Miller wrote on Mon, Feb 25, 2019 at 09:45:12AM -0700:
> >>> On Mon, 25 Feb 2019 12:39:41 +0100, Ingo Schwarze wrote:
>
> >>>> Index: line.c
> >> [...]
> >>>> @@ -469,11 +469,10 @@ in_ansi_esc_seq(void)
> >>>> * Search backwards for either an ESC (which means we ARE in a
> >>>> seq);
> >>>> * or an end char (which means we're NOT in a seq).
> >>>> */
> >>>> - for (p = &linebuf[curr]; p > linebuf; ) {
> >>>> - LWCHAR ch = step_char(&p, -1, linebuf);
> >>>> - if (IS_CSI_START(ch))
> >>>> + for (p = linebuf + curr - 1; p >= linebuf; p--) {
>
> >>> Since curr can be 0, can this lead to be a single byte underflow?
>
> >> No, in that case (which logically means the line buffer is empty),
> >> the end condition p >= linebuf is false right away, the loop
> >> is never entered, the function returns 0 right away and at the
> >> call site, the first if brach (containing "curr--") isn't entered
> >> either.
>
> > Strictly speaking, the result of "p = linebuf + curr - 1" is undefined
> > when curr < 1. There is a special case in the standard when the
> > result is one past the end of an array but no corresponding case
> > for one element before the array. In practice, it is unlikely to
> > matter.
>
> Ouch, i keep forgetting that one, thanks for reminding me.
>
> In that case, let's just use an index rather than a pointer;
> diff otherwise unchanged.
>
> OK?
> Ingo
>
>
> Index: cvt.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/cvt.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 cvt.c
> --- cvt.c 17 Sep 2016 15:06:41 -0000 1.8
> +++ cvt.c 26 Feb 2019 01:31:31 -0000
> @@ -77,7 +77,7 @@ cvt_text(char *odst, char *osrc, int *ch
> dst--;
> } while (dst > odst &&
> !IS_ASCII_OCTET(*dst) && !IS_UTF8_LEAD(*dst));
> - } else if ((ops & CVT_ANSI) && IS_CSI_START(ch)) {
> + } else if ((ops & CVT_ANSI) && ch == ESC) {
> /* Skip to end of ANSI escape sequence. */
> src++; /* skip the CSI start char */
> while (src < src_end)
> Index: filename.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/filename.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 filename.c
> --- filename.c 29 Oct 2017 17:10:55 -0000 1.26
> +++ filename.c 26 Feb 2019 01:31:31 -0000
> @@ -348,7 +348,7 @@ bin_file(int f)
> pend = &data[n];
> for (p = data; p < pend; ) {
> LWCHAR c = step_char(&p, +1, pend);
> - if (ctldisp == OPT_ONPLUS && IS_CSI_START(c)) {
> + if (ctldisp == OPT_ONPLUS && c == ESC) {
> do {
> c = step_char(&p, +1, pend);
> } while (p < pend && is_ansi_middle(c));
> Index: less.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/less.h,v
> retrieving revision 1.28
> diff -u -p -r1.28 less.h
> --- less.h 30 Dec 2018 23:09:58 -0000 1.28
> +++ less.h 26 Feb 2019 01:31:31 -0000
> @@ -38,8 +38,6 @@
> #define IS_SPACE(c) isspace((unsigned char)(c))
> #define IS_DIGIT(c) isdigit((unsigned char)(c))
>
> -#define IS_CSI_START(c) (((LWCHAR)(c)) == ESC || (((LWCHAR)(c)) == CSI))
> -
> #ifndef TRUE
> #define TRUE 1
> #endif
> @@ -151,9 +149,7 @@ struct textlist {
> #define AT_INDET (1 << 7) /* Indeterminate: either bold or
> underline */
>
> #define CONTROL(c) ((c)&037)
> -
> #define ESC CONTROL('[')
> -#define CSI ((unsigned char)'\233')
>
> #define S_INTERRUPT 01
> #define S_STOP 02
> Index: line.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/line.c,v
> retrieving revision 1.23
> diff -u -p -r1.23 line.c
> --- line.c 24 Feb 2019 04:54:36 -0000 1.23
> +++ line.c 26 Feb 2019 01:31:31 -0000
> @@ -230,7 +230,7 @@ pshift(int shift)
> */
> while (shifted <= shift && from < curr) {
> c = linebuf[from];
> - if (ctldisp == OPT_ONPLUS && IS_CSI_START(c)) {
> + if (ctldisp == OPT_ONPLUS && c == ESC) {
> /* Keep cumulative effect. */
> linebuf[to] = c;
> attr[to++] = attr[from++];
> @@ -463,17 +463,16 @@ backc(void)
> static int
> in_ansi_esc_seq(void)
> {
> - char *p;
> + int i;
>
> /*
> * Search backwards for either an ESC (which means we ARE in a seq);
> * or an end char (which means we're NOT in a seq).
> */
> - for (p = &linebuf[curr]; p > linebuf; ) {
> - LWCHAR ch = step_char(&p, -1, linebuf);
> - if (IS_CSI_START(ch))
> + for (i = curr - 1; i >= 0; i--) {
> + if (linebuf[i] == ESC)
> return (1);
> - if (!is_ansi_middle(ch))
> + if (!is_ansi_middle(linebuf[i]))
> return (0);
> }
> return (0);
> @@ -533,17 +532,14 @@ store_char(LWCHAR ch, char a, char *rep,
> if (ctldisp == OPT_ONPLUS && in_ansi_esc_seq()) {
> if (!is_ansi_end(ch) && !is_ansi_middle(ch)) {
> /* Remove whole unrecognized sequence. */
> - char *p = &linebuf[curr];
> - LWCHAR bch;
> do {
> - bch = step_char(&p, -1, linebuf);
> - } while (p > linebuf && !IS_CSI_START(bch));
> - curr = p - linebuf;
> + curr--;
> + } while (curr > 0 && linebuf[curr] != ESC);
> return (0);
> }
> a = AT_ANSI; /* Will force re-AT_'ing around it. */
> w = 0;
> - } else if (ctldisp == OPT_ONPLUS && IS_CSI_START(ch)) {
> + } else if (ctldisp == OPT_ONPLUS && ch == ESC) {
> a = AT_ANSI; /* Will force re-AT_'ing around it. */
> w = 0;
> } else {
> @@ -851,7 +847,7 @@ do_append(LWCHAR ch, char *rep, off_t po
> } else if ((!utf_mode || is_ascii_char(ch)) && control_char((char)ch)) {
> do_control_char:
> if (ctldisp == OPT_ON ||
> - (ctldisp == OPT_ONPLUS && IS_CSI_START(ch))) {
> + (ctldisp == OPT_ONPLUS && ch == ESC)) {
> /*
> * Output as a normal character.
> */
