Subject: Re: Mouse console support
To: Bang Jun-Young <junyoung@mogua.com>
From: Lennart Augustsson <lennart@augustsson.net>
List: tech-kern
Date: 02/01/2002 19:30:40
I feel that the general archtecture of the OpenBSD mouse stuff is
questionable.  They have decided to have both userland and
kernel stuff.  Fair enough, but once you have a userland daemon
I would expect that you put as much there as possible.  I got the
impression that the OpenBSD code had too much in the kernel (like
code to decide about what you are selecting).
Furthermore, it seems that this addition requires some addition to
every machine dependent wscons driver.  This is really a bad design.
Maybe wscons is designed that way, if it is, it should be changed
so that things like wsmoused can be added without MD code.

Maybe I'll have a look at the ws code some day, and give some more
constructive critisism.

    -- Lennart


Bang Jun-Young wrote:

> On Wed, Jan 30, 2002 at 07:58:37PM +0100, Julio Merino wrote:
> > Should I send it to the list for code veritication, or only to a
> > commiter?
>
> I had a look at the OpenBSD code before, and I came to the
> following conclusion: It shouldn't be committed as it is. Other
> than the problem I said in the previous mail, there are too many
> places in which variables of uppercase names are used in
> assignment statements:
>
>         CPY_START = row * N_COLS;
>         CPY_END = CPY_START + (N_COLS - 1);
>         ...
>         MOUSE_FLAGS |= MOUSE_VISIBLE;
>
> When I first saw the code, I felt like I was looking at 70's BASIC
> source. IMHO, in the tradition of C only constants or variables of
> which values are not modified should be written in uppercases (note
> I'm not saying macro functions). They are defined in wsmoused.h as
> follows:
>
> #define CPY_START       (sc->sc_focus->cpy_start)
> #define MOUSE_FLAGS     (sc->sc_focus->mouse_flags)
>
> It would be good if those names were converted to lowercase ones and
> prefixed with wsm_ or something like that.
>
> Also, it should be cleanly separated from the rest of sources so
> that it is possible to compile optionally, i.e. options WSCONS_WSMOUSED
> in the kernel config file, if considering there are quite a number
> of people who issue objection to any new features that might make their
> kernels bigger, even if it's relatively small and quite useful (most of
> them are *real* i386 users, I guess).
>
> Jun-Young
>
> --
> Bang Jun-Young <junyoung@mogua.com>