[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4D272D55.3040807@suse.cz>
Date: Fri, 07 Jan 2011 16:12:21 +0100
From: Jiri Slaby <jslaby@...e.cz>
To: Kay Sievers <kay.sievers@...y.org>
CC: Greg Kroah-Hartman <gregkh@...e.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 34/36] tty: add 'active' sysfs attribute to tty0 and console
device
On 01/07/2011 02:20 PM, Kay Sievers wrote:
> On Fri, Jan 7, 2011 at 10:09, Jiri Slaby <jslaby@...e.cz> wrote:
>> On 01/06/2011 11:23 PM, Greg Kroah-Hartman wrote:
>>> --- a/drivers/tty/tty_io.c
>>> +++ b/drivers/tty/tty_io.c
>>> @@ -3232,9 +3232,45 @@ static int __init tty_class_init(void)
>>> postcore_initcall(tty_class_init);
>>>
>>> /* 3/2004 jmc: why do these devices exist? */
>>> -
>>> static struct cdev tty_cdev, console_cdev;
>>>
>>> +static ssize_t show_cons_active(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct console *cs[16];
>>> + int i = 0;
>>> + struct console *c;
>>> + ssize_t count = 0;
>>> +
>>> + acquire_console_sem();
>>> + for (c = console_drivers; c; c = c->next) {
>>
>> This should be:
>> for_each_console(c)
>> like somebody already suggested.
>
> That's exported now? It was local to printk.c at least that time.
>
> Anyway, the only sensible change here is to convert that home-grown
> ->next thing to a proper kernel linked list, and use the common
> iterators instead of doing it on our own, or export new custom ones.
Perhaps -- I didn't look into the code if there is something which
blocked this to be done years ago. I doubt there is...
>>> + if (!c->device)
>>> + continue;
>>
>> Why are consoles without devices ignored here?
>
> It's a common pattern all over the place. What is a "console without a
> device" that would be interesting here?
For example parport console. The question is whether you are interested
in consoles without ttys in systemd and other potential users.
>> Other than that there are 2 build warnings, see below.
>>
>>> - device_create(tty_class, NULL, MKDEV(TTY_MAJOR, 0), NULL, "tty0");
>>> + tty0dev = device_create(tty_class, NULL, MKDEV(TTY_MAJOR, 0), NULL, "tty0");
>>> + if (IS_ERR(tty0dev))
>>> + tty0dev = NULL;
>>> + else
>>> + device_create_file(tty0dev, &dev_attr_active);
>>
>> drivers/tty/vt/vt.c: In function ‘vty_init’:
>> drivers/tty/vt/vt.c:2997:21: warning: ignoring return value of
>> ‘device_create_file’, declared with attribute warn_unused_result
>
> The code doesn't even check device_create(). :)
>
> Anyway we actually don't want to check the return value of
> file_create(), it's a non-fatal error, if it ever happens.
Ok, but we don't want to introduce new warnings.
> What's the preferred way to do that:
> int ignore = device_create_file()
I think the usual workaround in C is casting to void.
regards,
--
js
suse labs
--
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/
Powered by blists - more mailing lists