Messages in this thread | | | Date | Tue, 2 Apr 2024 13:02:20 +0200 | Subject | Re: [RESEND PATCH v3 1/2] VT: Add KDFONTINFO ioctl | From | Jiri Slaby <> |
| |
Hi,
On 02. 04. 24, 12:32, Alexey Gladkov wrote: > Each driver has its own restrictions on font size. There is currently no > way to understand what the requirements are. The new ioctl allows > userspace to get the minmum and maximum font size values.
minimum
> Acked-by: Helge Deller <deller@gmx.de> > Signed-off-by: Alexey Gladkov <legion@kernel.org> > --- > drivers/tty/vt/vt.c | 24 ++++++++++++++++++++++++ > drivers/tty/vt/vt_ioctl.c | 13 +++++++++++++ > include/linux/console.h | 2 ++ > include/linux/vt_kern.h | 1 + > include/uapi/linux/kd.h | 13 ++++++++++++- > 5 files changed, 52 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c > index 156efda7c80d..8c2a3d98b5ec 100644 > --- a/drivers/tty/vt/vt.c > +++ b/drivers/tty/vt/vt.c > @@ -4680,6 +4680,30 @@ int con_font_op(struct vc_data *vc, struct console_font_op *op) > return -ENOSYS; > } > > +int con_font_info(struct vc_data *vc, struct console_font_info *info) > +{ > + int rc = -EINVAL;
This initialization appears to be unneeded.
> + > + info->min_height = 0; > + info->max_height = max_font_height; > + > + info->min_width = 0; > + info->max_width = max_font_width; > + > + info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE; > + > + console_lock(); > + if (vc->vc_mode != KD_TEXT) > + rc = -EINVAL; > + else if (vc->vc_sw->con_font_info) > + rc = vc->vc_sw->con_font_info(vc, info); > + else > + rc = -ENOSYS; > + console_unlock(); > + > + return rc; > +} > + > /* > * Interface exported to selection and vcs. > */ > diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c > index 8c685b501404..b3b4e4b69366 100644 > --- a/drivers/tty/vt/vt_ioctl.c > +++ b/drivers/tty/vt/vt_ioctl.c > @@ -479,6 +479,19 @@ static int vt_k_ioctl(struct tty_struct *tty, unsigned int cmd, > break; > } > > + case KDFONTINFO: { > + struct console_font_info fnt_info; > + > + if (copy_from_user(&fnt_info, up, sizeof(fnt_info))) > + return -EFAULT;
Who uses the copied values?
> + ret = con_font_info(vc, &fnt_info); > + if (ret) > + return ret; > + if (copy_to_user(up, &fnt_info, sizeof(fnt_info)))
We should do the preferred sizeof(*up) here...
> + return -EFAULT; > + break; > + } > + > default: > return -ENOIOCTLCMD; > } .. > --- a/include/uapi/linux/kd.h > +++ b/include/uapi/linux/kd.h > @@ -183,8 +183,19 @@ struct console_font { > > #define KD_FONT_FLAG_DONT_RECALC 1 /* Don't recalculate hw charcell size [compat] */ > > +#define KDFONTINFO 0x4B73 /* font information */
Why not properly define the number using IOC() et al.? K (that 0x4b) is even reserved for kd.h.
> +#define KD_FONT_INFO_FLAG_LOW_SIZE (1U << 0) /* 256 */ > +#define KD_FONT_INFO_FLAG_HIGH_SIZE (1U << 1) /* 512 */
_BITUL()
> +struct console_font_info { > + unsigned int min_width, min_height; /* minimal font size */ > + unsigned int max_width, max_height; /* maximum font size */ > + unsigned int flags; /* KD_FONT_INFO_FLAG_* */
This does not look like a well-defined™ and extendable uapi structure. While it won't change anything here, still use fixed-length __u32.
And you should perhaps add some reserved fields. Do not repeat the same mistakes as your predecessors with the current kd uapi.
> +};
thanks, -- js suse labs
| |