Subject: Re: Fix for get/putwschar
To: None <tech-kern@netbsd.org>
From: Valeriy E. Ushakov <uwe@ptc.spbu.ru>
List: tech-kern
Date: 04/13/2006 23:56:31
On Thu, Apr 13, 2006 at 19:32:00 +0200, Julio M. Merino Vidal wrote:
> Attached is a patch to fix the get/putwschar functions. It:
>
> - Removes the getwschar and putwschar accessops from the wsdisplay
> devices.
[...]
> - Gets rid of the WSDISPLAY_CHARFUNCS kernel option; useless.
As these changes are pretty mechanical, it would be very helpful if
they were in a separate file to make review easier.
> Index: dev/ic/vga.c
> @@ -771,6 +760,7 @@ int
> vga_ioctl(void *v, void *vs, u_long cmd, caddr_t data, int flag, struct lwp *l)
> {
> struct vga_config *vc = v;
> + struct vgascreen *scr = vs;
> const struct vga_funcs *vf = vc->vc_funcs;
>
> switch (cmd) {
> @@ -791,6 +781,14 @@ vga_ioctl(void *v, void *vs, u_long cmd,
> vga_set_video(vc, *(int *)data == WSDISPLAYIO_VIDEO_ON);
> return 0;
>
> + case WSDISPLAYIO_GETWSCHAR:
> + return pcdisplay_getwschar((struct pcdisplayscreen *)scr,
> + (struct wsdisplay_char *)data);
Please, don't use casts like thas. &scr->pcs is shorter and is easier
to read and understand (see below).
> +
> + case WSDISPLAYIO_PUTWSCHAR:
> + return pcdisplay_putwschar((struct pcdisplayscreen *)scr,
> + (struct wsdisplay_char *)data);
Ditto.
> #ifdef WSDISPLAY_CUSTOM_BORDER
> case WSDISPLAYIO_GBORDER:
> return (vga_getborder(vc, (u_int *)data));
> @@ -1417,27 +1415,6 @@ vga_putchar(void *c, int row, int col, u
> pcdisplay_putchar(c, row, col, uc, attr);
> }
>
> -
> -#ifdef WSDISPLAY_CHARFUNCS
> -int
> -vga_getwschar(void *cookie, struct wsdisplay_char *wschar)
> -{
> - struct vgascreen *scr = cookie;
> -
> - if (scr == NULL) return 0;
> - return (pcdisplay_getwschar(&scr->pcs, wschar));
This is the correct way.
> Index: dev/wscons/wsdisplay_vcons.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/wscons/wsdisplay_vcons.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 wsdisplay_vcons.c
> --- dev/wscons/wsdisplay_vcons.c 19 Feb 2006 03:51:03 -0000 1.5
> +++ dev/wscons/wsdisplay_vcons.c 13 Apr 2006 17:27:43 -0000
> @@ -60,6 +60,7 @@ __KERNEL_RCSID(0, "$NetBSD: wsdisplay_vc
> static void vcons_dummy_init_screen(void *, struct vcons_screen *, int,
> long *);
>
> +static int vcons_ioctl(void *, void *, u_long, caddr_t, int, struct lwp *);
> static int vcons_alloc_screen(void *, const struct wsscreen_descr *, void **,
> int *, int *, long *);
> static void vcons_free_screen(void *, void *);
> @@ -87,8 +88,8 @@ static void vcons_putchar(void *, int, i
> static void vcons_cursor(void *, int, int, int);
>
> /* support for readin/writing text buffers. For wsmoused */
> -static int vcons_putwschar(void *, struct wsdisplay_char *);
> -static int vcons_getwschar(void *, struct wsdisplay_char *);
> +static int vcons_putwschar(struct rasops_info *, struct wsdisplay_char *);
> +static int vcons_getwschar(struct rasops_info *, struct wsdisplay_char *);
Is rasops_info semantically correct way to refer to a vcons screen?
Admittedly I haven't checked it thoroughly, but I suspect - no.
Michael?
> @@ -361,6 +366,34 @@ vcons_redraw_screen(struct vcons_screen
> }
>
> static int
> +vcons_ioctl(void *v, void *vs, u_long cmd, caddr_t data, int flag,
> + struct lwp *l)
> +{
> + struct vcons_data *vd = v;
> + int error;
> +
> + switch (cmd) {
> + case WSDISPLAYIO_GETWSCHAR:
> + error = vcons_getwschar(vd->cookie,
> + (struct wsdisplay_char *)data);
> + break;
vd->cookie is driver's cookie to vcons. It cannot be rasops (see
prototype in the previous hunk). E.g. isgfb passes struct
igsfb_devconfig as cookie.
> +
> + case WSDISPLAYIO_PUTWSCHAR:
> + error = vcons_putwschar(vd->cookie,
> + (struct wsdisplay_char *)data);
> + break;
Ditto.
> +
> + default:
> + if (vd->ioctl != NULL)
> + error = (*vd->ioctl)(v, vs, cmd, data, flag, l);
> + else
> + error = EINVAL;
> + }
> +
> + return error;
> +}
Use EPASSTHROUGH instead. Upper layers will handle it.
SY, Uwe
--
uwe@ptc.spbu.ru | Zu Grunde kommen
http://snark.ptc.spbu.ru/~uwe/ | Ist zu Grunde gehen