Hi Jiri,
thanks for your patience ... ;-)
Am Do., 2. Juli 2020 um 10:48 Uhr schrieb Jiri Slaby <jirisl...@kernel.org>:
>
> On 01. 07. 20, 17:13, Tammo Block wrote:
> > The SRG protocol indicates a button release by appending a "m" to the
> > report. In this case the button number is not 3 (RELEASEEVENT) but
> > the number of the button that was released. As release event are only
> > reported for the first three buttons (LOWBUTTONMASK), we need to store
> > the number on click events because it is not sent to us from userspace.
> >
> > We also need to check for the case where no button state change occurred
> > at all (bit 6 set), in this case a value of 3 is OK even in SRG.
> >
> > Signed-off-by: Tammo Block <tammo.bl...@gmail.com>
> > ---
> > drivers/tty/vt/vt.c | 25 ++++++++++++++++++++++---
> > 1 file changed, 22 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> > index 9abf2829b1d3..9aae3eac7989 100644
> > --- a/drivers/tty/vt/vt.c
> > +++ b/drivers/tty/vt/vt.c
> > @@ -1838,15 +1838,34 @@ static inline void respond_ID(struct tty_struct
> > *tty)
> > respond_string(vt102_id, strlen(vt102_id), tty->port);
> > }
> >
> > +#define ANYBUTTONMASK 0xc3
> > +#define LOWBUTTONMASK 0x03
>
> Insert _ before MASK.
>
> > +#define RELEASEEVENT 0x03
> > +
> > enum { Mouse_X10 = 0, Mouse_SRG, Mouse_URXVT};
>
> = 0 in unnecessary. And put one per line. And all capitals as
> CodingStyle says.
>
OK, I was just copying the style of other enums in vt.c.
But the code seems to be older than the coding style guide ... ;-)
> You should name the enum somehow and use it as a type for
> vc_protocol_mouse (you'd need a forward declaration of the enum in the
> header).
>
That would make vc_protocol_mouse an enum and vc_report_mouse a define.
As vc_report_mouse is visible from userspace those defines live in tiocl.h.
Wouldn't it be more consistent to make both of them the same type and also use
defines for vc_report_mouse? And if so: Where to place them?
They are only needed inside vt.c, so they could live in any of:
1.) Inside vt.c
2.) in vt.h
3.) in tiocl.h, although userspace does not need them.
> > void mouse_report(struct tty_struct *tty, int butt, int mrx, int mry)
> > {
> > - char buf[8];
> > + static char last_btn = RELEASEEVENT;
> > + char buf[20];
> > + bool rel;
> > int len;
> >
> > - len = sprintf(buf, "\033[M%c%c%c", (char)(' ' + butt),
> > - (char)('!' + mrx), (char)('!' + mry));
> > + switch (vc_cons[fg_console].d->vc_protocol_mouse) {
> > + case Mouse_SRG:
>
> This is not how we indent switch-case.
>
> > + rel = (butt & ANYBUTTONMASK) == RELEASEEVENT;
> > + if ((butt & ANYBUTTONMASK) < RELEASEEVENT)
> > + last_btn = butt & LOWBUTTONMASK;
> > + if ((butt & TIOCL_SELBUTTONMASK) == RELEASEEVENT)
> > + butt = (butt & ~LOWBUTTONMASK) | last_btn;
> > + len = sprintf(buf, "\033[<%d;%d;%d%c", butt,
> > + mrx + 1, mry + 1, rel ? 'm' : 'M');
> > + break;
> > + default:
> > + len = sprintf(buf, "\033[M%c%c%c", (char)(' ' + butt),
> > + (char)('!' + mrx), (char)('!' + mry));
> > + break;
> > + }
> > respond_string(buf, len, tty->port);
> > }
>
> thanks,
> --
> js
> suse labs
Thanks again,
Tammo
