linux-kernel - Re: [PATCH2] Basic braille screen reader support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080223131028.GC4244@implementation>
Date:	Sat, 23 Feb 2008 14:10:28 +0100
From:	Samuel Thibault <samuel.thibault@...-lyon.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org, Dmitry Torokhov <dtor@...l.ru>,
	Jiri Kosina <jkosina@...e.cz>
Subject: Re: [PATCH2] Basic braille screen reader support

Andrew Morton, le Sat 23 Feb 2008 00:04:10 -0800, a écrit :
> > +++ linux-2.6.24.1-perso/kernel/printk.c	2008-02-21 12:09:06.000000000 +0100
> > @@ -105,6 +105,7 @@
> 
> Please use `diff -p'

Argl, that's editdiff's fault.  We need to fix it to keep the -p
comments.

> >  	char	name[8];			/* Name of the driver	    */
> >  	int	index;				/* Minor dev. to use	    */
> >  	char	*options;			/* Options for the driver   */
> > +	char	*brl_options;			/* Options for braille driver */
> >  };
> 
> Should this depend on CONFIG_A11Y_BRAILLE_CONSOLE or whatever?

It can, indeed.  It just makes the code a little more clumsy to save 32
or 64 bytes.

> > +	for (i = 0; i < MAX_CMDLINECONSOLES && console_cmdline[i].name[0]; i++)
> > +		if (strcmp(console_cmdline[i].name, name) == 0 &&
> > +			  console_cmdline[i].index == idx) {
> > +				if (!brl_options)
> > +					selected_console = i;
> > +				return 0;
> > +		}
> > +	if (i == MAX_CMDLINECONSOLES)
> > +		return -E2BIG;
> 
> Does the array of consles have any locking here?

Well, that's actually just the original code that I moved a bit earlier,
so the code path is not changed.

> > +	if (!brl_options)
> > +		selected_console = i;
> > +	c = &console_cmdline[i];
> > +	memcpy(c->name, name, sizeof(c->name));
> > +	c->name[sizeof(c->name) - 1] = 0;
> 
> strlcpy()?

Again, that's the original code, unmodified, but we can put an strlcpy
here yes.

> > +static void beep(unsigned int freq)
> > +{
> > +	if (sound)
> > +		kd_mksound(freq, HZ/10);
> > +}
> 
> hm, do we have enough Kconfig dependencies here to ensure that kd_mksound()
> is always available?

Ah, CONFIG_VT is needed indeed.

I'll address those in replies to this mail.

Samuel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/