| 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
| ||
|
Date: Tue, 9 Sep 2008 09:40:18 +0200
From: Sebastian Siewior <lkml@...breakpoint.cc>
To: Jarod Wilson <jwilson@...hat.com>
Cc: linux-kernel@...r.kernel.org, Jarod Wilson <jarod@...hat.com>,
Janne Grunau <j@...nau.net>,
Christoph Bartelmus <lirc@...telmus.de>,
Mario Limonciello <superm1@...ntu.com>
Subject: Re: [PATCH 01/18] lirc core device driver infrastructure
>diff --git a/drivers/input/lirc/Makefile b/drivers/input/lirc/Makefile
>new file mode 100644
>index 0000000..cdb4c45
>--- /dev/null
>+++ b/drivers/input/lirc/Makefile
>@@ -0,0 +1,8 @@
>+# Makefile for the lirc drivers.
>+#
>+
>+# Each configuration option enables a list of files.
>+
>+EXTRA_CFLAGS =-DIRCTL_DEV_MAJOR=61 -DLIRC_SERIAL_TRANSMITTER -I$(src)
Do you rely on this specific major? Since your daemon opens /dev/lirc0
you don't need a fixed major do you?
LIRC_SERIAL_TRANSMITTER is used in patch 2 and just to enable a module
options. Since it is always the case, please remove it.
I haven't found the source of $src. It is probably a relic.
>+obj-$(CONFIG_LIRC_DEV) += lirc_dev.o
>diff --git a/drivers/input/lirc/lirc_dev.c b/drivers/input/lirc/lirc_dev.c
>new file mode 100644
>index 0000000..c8f325c
>--- /dev/null
>+++ b/drivers/input/lirc/lirc_dev.c
>@@ -0,0 +1,809 @@
>+/*
>+ * LIRC base driver
>+ *
>+ * (L) by Artur Lipowski <alipowski@...eria.pl>
Is that L here on purpose?
>+ *
>+ * This program is free software; you can redistribute it and/or modify
>+ * it under the terms of the GNU General Public License as published by
>+ * the Free Software Foundation; either version 2 of the License, or
>+ * (at your option) any later version.
>+ *
>+ * This program is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+ * GNU General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU General Public License
>+ * along with this program; if not, write to the Free Software
>+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+ *
>+ */
>+
>+#include <linux/version.h>
You souldn't need this one
>+
>+#include <linux/autoconf.h>
this as well.
>+#include <linux/module.h>
>+#include <linux/kernel.h>
>+#include <linux/sched.h>
>+#include <linux/errno.h>
>+#include <linux/ioctl.h>
>+#include <linux/fs.h>
>+#include <linux/poll.h>
>+#include <linux/smp_lock.h>
if you need this than you use the BKL back. As far as I remember
the ioctl() handler in kernel core no longer takes the BKL and I don't
see any locking in irctl_ioctl().
>+#include <linux/completion.h>
>+#include <linux/uaccess.h>
>+#include <linux/errno.h>
>+#include <linux/semaphore.h>
haven't found any call of down(). Do you really need this?
>+#define __KERNEL_SYSCALLS__
this define shouldn't be here, should it?
>+#include <linux/unistd.h>
>+#include <linux/kthread.h>
>+
>+/* SysFS header */
>+#include <linux/device.h>
>+
>+#include "lirc.h"
>+#include "lirc_dev.h"
>+
>+static int debug;
>+#define dprintk(fmt, args...) \
>+ do { \
>+ if (debug) \
>+ printk(KERN_DEBUG fmt, ## args); \
>+ } while (0)
>+
>+#define IRCTL_DEV_NAME "BaseRemoteCtl"
>+#define SUCCESS 0
>+#define NOPLUG -1
>+#define LOGHEAD "lirc_dev (%s[%d]): "
>+
>+struct irctl {
>+ struct lirc_plugin p;
>+ int attached;
>+ int open;
>+
>+ struct mutex buffer_lock;
>+ struct lirc_buffer *buf;
>+
>+ struct task_struct *task;
>+ long jiffies_to_wait;
>+
>+};
>+
>+static DEFINE_MUTEX(plugin_lock);
>+
>+static struct irctl irctls[MAX_IRCTL_DEVICES];
>+static struct file_operations fops;
>+
>+/* Only used for sysfs but defined to void otherwise */
>+static struct class *lirc_class;
>+
>+/* helper function
>+ * initializes the irctl structure
>+ */
For all comments above functions:
- Please use the default comment style.
- don't comment obvious things
- please use kernel doc if
- please comment the prototypes but the actual function.
>+static inline void init_irctl(struct irctl *ir)
this inline can go in my opinion. The others here as well. It is not
*that* performance critical.
>+{
>+ memset(&ir->p, 0, sizeof(struct lirc_plugin));
>+ mutex_init(&ir->buffer_lock);
>+ ir->p.minor = NOPLUG;
>+
>+ ir->task = NULL;
>+ ir->jiffies_to_wait = 0;
>+
>+ ir->open = 0;
>+ ir->attached = 0;
>+}
>+
>+static void cleanup(struct irctl *ir)
>+{
>+ dprintk(LOGHEAD "cleaning up\n", ir->p.name, ir->p.minor);
>+
>+ device_destroy(lirc_class, MKDEV(IRCTL_DEV_MAJOR, ir->p.minor));
>+
>+ if (ir->buf != ir->p.rbuf) {
>+ lirc_buffer_free(ir->buf);
>+ kfree(ir->buf);
>+ }
>+ ir->buf = NULL;
>+
>+ init_irctl(ir);
>+}
>+
>+/* helper function
>+ * reads key codes from plugin and puts them into buffer
>+ * buffer free space is checked and locking performed
>+ * returns 0 on success
>+ */
>+static inline int add_to_buf(struct irctl *ir)
>+{
>+ if (lirc_buffer_full(ir->buf)) {
>+ dprintk(LOGHEAD "buffer overflow\n",
>+ ir->p.name, ir->p.minor);
>+ return -EOVERFLOW;
>+ }
>+
>+ if (ir->p.add_to_buf) {
>+ int res = -ENODATA;
>+ int got_data = 0;
>+
>+ /* service the device as long as it is returning
>+ * data and we have space
>+ */
>+ while (!lirc_buffer_full(ir->buf)) {
>+ res = ir->p.add_to_buf(ir->p.data, ir->buf);
>+ if (res == SUCCESS)
>+ got_data++;
>+ else
>+ break;
>+ }
>+
>+ if (res == -ENODEV)
>+ kthread_stop(ir->task);
>+
>+ return got_data ? SUCCESS : res;
>+ }
>+
>+ return SUCCESS;
>+}
>+
>+/* main function of the polling thread
>+ */
>+static int lirc_thread(void *irctl)
>+{
>+ struct irctl *ir = irctl;
>+
>+ /* This thread doesn't need any user-level access,
>+ * so get rid of all our resources
>+ */
>+
>+ dprintk(LOGHEAD "poll thread started\n", ir->p.name, ir->p.minor);
>+
>+ do {
>+ if (ir->open) {
>+ if (ir->jiffies_to_wait) {
>+ set_current_state(TASK_INTERRUPTIBLE);
>+ schedule_timeout(ir->jiffies_to_wait);
>+ } else {
>+ interruptible_sleep_on(
>+ ir->p.get_queue(ir->p.data));
>+ }
>+ if (kthread_should_stop())
>+ break;
>+ if (!add_to_buf(ir))
>+ wake_up_interruptible(&ir->buf->wait_poll);
>+ } else {
>+ /* if device not opened so we can sleep half a second */
>+ set_current_state(TASK_INTERRUPTIBLE);
>+ schedule_timeout(HZ/2);
>+ }
>+ } while (!kthread_should_stop());
>+
>+ dprintk(LOGHEAD "poll thread ended\n", ir->p.name, ir->p.minor);
>+
>+ return 0;
>+}
>+
>+int lirc_register_plugin(struct lirc_plugin *p)
>+{
>+ struct irctl *ir;
>+ int minor;
>+ int bytes_in_key;
>+ int err;
>+ DECLARE_COMPLETION(tn);
>+
>+ if (!p) {
>+ printk(KERN_ERR "lirc_dev: lirc_register_plugin: "
>+ "plugin pointer must be not NULL!\n");
>+ err = -EBADRQC;
>+ goto out;
>+ }
>+
>+ if (MAX_IRCTL_DEVICES <= p->minor) {
>+ printk(KERN_ERR "lirc_dev: lirc_register_plugin: "
>+ "\"minor\" must be between 0 and %d (%d)!\n",
>+ MAX_IRCTL_DEVICES-1, p->minor);
>+ err = -EBADRQC;
>+ goto out;
>+ }
>+
>+ if (1 > p->code_length || (BUFLEN * 8) < p->code_length) {
>+ printk(KERN_ERR "lirc_dev: lirc_register_plugin: "
>+ "code length in bits for minor (%d) "
>+ "must be less than %d!\n",
>+ p->minor, BUFLEN * 8);
>+ err = -EBADRQC;
>+ goto out;
>+ }
>+
>+ printk(KERN_INFO "lirc_dev: lirc_register_plugin: sample_rate: %d\n",
>+ p->sample_rate);
>+ if (p->sample_rate) {
>+ if (2 > p->sample_rate || HZ < p->sample_rate) {
>+ printk(KERN_ERR "lirc_dev: lirc_register_plugin: "
>+ "sample_rate must be between 2 and %d!\n", HZ);
>+ err = -EBADRQC;
>+ goto out;
>+ }
>+ if (!p->add_to_buf) {
>+ printk(KERN_ERR "lirc_dev: lirc_register_plugin: "
>+ "add_to_buf cannot be NULL when "
>+ "sample_rate is set\n");
>+ err = -EBADRQC;
>+ goto out;
>+ }
>+ } else if (!(p->fops && p->fops->read)
>+ && !p->get_queue && !p->rbuf) {
>+ printk(KERN_ERR "lirc_dev: lirc_register_plugin: "
>+ "fops->read, get_queue and rbuf "
>+ "cannot all be NULL!\n");
>+ err = -EBADRQC;
>+ goto out;
>+ } else if (!p->get_queue && !p->rbuf) {
>+ if (!(p->fops && p->fops->read && p->fops->poll)
>+ || (!p->fops->ioctl && !p->ioctl)) {
>+ printk(KERN_ERR "lirc_dev: lirc_register_plugin: "
>+ "neither read, poll nor ioctl can be NULL!\n");
>+ err = -EBADRQC;
>+ goto out;
>+ }
>+ }
>+
>+ if (p->owner == NULL) {
>+ printk(KERN_ERR "lirc_dev: lirc_register_plugin: "
>+ "no module owner registered\n");
>+ err = -EBADRQC;
>+ goto out;
>+ }
>+
>+ mutex_lock(&plugin_lock);
>+
>+ minor = p->minor;
>+
>+ if (0 > minor) {
>+ /* find first free slot for plugin */
>+ for (minor = 0; minor < MAX_IRCTL_DEVICES; minor++)
>+ if (irctls[minor].p.minor == NOPLUG)
>+ break;
>+ if (MAX_IRCTL_DEVICES == minor) {
>+ printk(KERN_ERR "lirc_dev: lirc_register_plugin: "
>+ "no free slots for plugins!\n");
>+ err = -ENOMEM;
>+ goto out_lock;
>+ }
>+ } else if (irctls[minor].p.minor != NOPLUG) {
>+ printk(KERN_ERR "lirc_dev: lirc_register_plugin: "
>+ "minor (%d) just registered!\n", minor);
>+ err = -EBUSY;
>+ goto out_lock;
>+ }
>+
>+ ir = &irctls[minor];
>+
>+ if (p->sample_rate) {
>+ ir->jiffies_to_wait = HZ / p->sample_rate;
>+ } else {
>+ /* it means - wait for external event in task queue */
>+ ir->jiffies_to_wait = 0;
>+ }
>+
>+ /* some safety check 8-) */
>+ p->name[sizeof(p->name)-1] = '\0';
>+
>+ bytes_in_key = p->code_length/8 + (p->code_length%8 ? 1 : 0);
did you actually pass checkpatch.pl ?
>+
>+ if (p->rbuf) {
>+ ir->buf = p->rbuf;
>+ } else {
>+ ir->buf = kmalloc(sizeof(struct lirc_buffer), GFP_KERNEL);
>+ if (!ir->buf) {
>+ err = -ENOMEM;
>+ goto out_lock;
>+ }
>+ if (lirc_buffer_init(ir->buf, bytes_in_key,
>+ BUFLEN/bytes_in_key) != 0) {
>+ kfree(ir->buf);
>+ err = -ENOMEM;
>+ goto out_lock;
>+ }
>+ }
>+
>+ if (p->features == 0)
>+ p->features = (p->code_length > 8) ?
>+ LIRC_CAN_REC_LIRCCODE : LIRC_CAN_REC_CODE;
>+
>+ ir->p = *p;
>+ ir->p.minor = minor;
>+
>+ device_create(lirc_class, ir->p.dev,
>+ MKDEV(IRCTL_DEV_MAJOR, ir->p.minor), NULL,
>+ "lirc%u", ir->p.minor);
>+
>+ if (p->sample_rate || p->get_queue) {
>+ /* try to fire up polling thread */
>+ ir->task = kthread_run(lirc_thread, (void *)ir, "lirc_dev");
>+ if (IS_ERR(ir->task)) {
>+ printk(KERN_ERR "lirc_dev: lirc_register_plugin: "
>+ "cannot run poll thread for minor = %d\n",
>+ p->minor);
>+ err = -ECHILD;
>+ goto out_sysfs;
>+ }
>+ }
>+ ir->attached = 1;
>+ mutex_unlock(&plugin_lock);
>+
>+/*
>+ * Recent kernels should handle this autmatically by increasing/decreasing
>+ * use count when a dependant module is loaded/unloaded.
>+ */
>+ dprintk("lirc_dev: plugin %s registered at minor number = %d\n",
>+ ir->p.name, ir->p.minor);
>+ p->minor = minor;
>+ return minor;
>+
>+out_sysfs:
>+ device_destroy(lirc_class, MKDEV(IRCTL_DEV_MAJOR, ir->p.minor));
>+out_lock:
>+ mutex_unlock(&plugin_lock);
>+out:
>+ return err;
>+}
>+EXPORT_SYMBOL(lirc_register_plugin);
Is EXPORT_SYMBOL_GPL() possible?
>+
>+int lirc_unregister_plugin(int minor)
>+{
>+ struct irctl *ir;
>+ DECLARE_COMPLETION(tn);
>+ DECLARE_COMPLETION(tn2);
>+
>+ if (minor < 0 || minor >= MAX_IRCTL_DEVICES) {
>+ printk(KERN_ERR "lirc_dev: lirc_unregister_plugin: "
>+ "\"minor\" must be between 0 and %d!\n",
>+ MAX_IRCTL_DEVICES-1);
>+ return -EBADRQC;
>+ }
>+
>+ ir = &irctls[minor];
>+
>+ mutex_lock(&plugin_lock);
>+
>+ if (ir->p.minor != minor) {
>+ printk(KERN_ERR "lirc_dev: lirc_unregister_plugin: "
>+ "minor (%d) device not registered!", minor);
>+ mutex_unlock(&plugin_lock);
>+ return -ENOENT;
>+ }
>+
>+ /* end up polling thread */
>+ if (ir->task) {
>+ wake_up_process(ir->task);
>+ kthread_stop(ir->task);
>+ }
>+
>+ dprintk("lirc_dev: plugin %s unregistered from minor number = %d\n",
>+ ir->p.name, ir->p.minor);
>+
>+ ir->attached = 0;
>+ if (ir->open) {
>+ dprintk(LOGHEAD "releasing opened plugin\n",
>+ ir->p.name, ir->p.minor);
>+ wake_up_interruptible(&ir->buf->wait_poll);
>+ mutex_lock(&ir->buffer_lock);
>+ ir->p.set_use_dec(ir->p.data);
>+ module_put(ir->p.owner);
>+ mutex_unlock(&ir->buffer_lock);
>+ } else
>+ cleanup(ir);
>+ mutex_unlock(&plugin_lock);
>+
>+/*
>+ * Recent kernels should handle this autmatically by increasing/decreasing
>+ * use count when a dependant module is loaded/unloaded.
>+ */
>+
>+ return SUCCESS;
>+}
>+EXPORT_SYMBOL(lirc_unregister_plugin);
>+
>+/*
>+ *
>+ */
>+static int irctl_open(struct inode *inode, struct file *file)
>+{
>+ struct irctl *ir;
>+ int retval;
>+
>+ if (MINOR(inode->i_rdev) >= MAX_IRCTL_DEVICES) {
>+ dprintk("lirc_dev [%d]: open result = -ENODEV\n",
>+ MINOR(inode->i_rdev));
>+ return -ENODEV;
>+ }
>+
>+ ir = &irctls[MINOR(inode->i_rdev)];
>+
>+ dprintk(LOGHEAD "open called\n", ir->p.name, ir->p.minor);
>+
>+ /* if the plugin has an open function use it instead */
>+ if (ir->p.fops && ir->p.fops->open)
>+ return ir->p.fops->open(inode, file);
>+
>+ if (mutex_lock_interruptible(&plugin_lock))
>+ return -ERESTARTSYS;
>+
>+ if (ir->p.minor == NOPLUG) {
>+ mutex_unlock(&plugin_lock);
>+ dprintk(LOGHEAD "open result = -ENODEV\n",
>+ ir->p.name, ir->p.minor);
>+ return -ENODEV;
>+ }
>+
>+ if (ir->open) {
>+ mutex_unlock(&plugin_lock);
>+ dprintk(LOGHEAD "open result = -EBUSY\n",
>+ ir->p.name, ir->p.minor);
>+ return -EBUSY;
>+ }
>+
>+ /* there is no need for locking here because ir->open is 0
>+ * and lirc_thread isn't using buffer
>+ * plugins which use irq's should allocate them on set_use_inc,
>+ * so there should be no problem with those either.
>+ */
>+ ir->buf->head = ir->buf->tail;
>+ ir->buf->fill = 0;
>+
>+ if (ir->p.owner != NULL && try_module_get(ir->p.owner)) {
>+ ++ir->open;
>+ retval = ir->p.set_use_inc(ir->p.data);
>+
>+ if (retval != SUCCESS) {
>+ module_put(ir->p.owner);
>+ --ir->open;
>+ }
>+ } else {
>+ if (ir->p.owner == NULL)
>+ dprintk(LOGHEAD "no module owner!!!\n",
>+ ir->p.name, ir->p.minor);
>+
>+ retval = -ENODEV;
>+ }
>+
>+ dprintk(LOGHEAD "open result = %d\n", ir->p.name, ir->p.minor, retval);
>+ mutex_unlock(&plugin_lock);
>+
>+ return retval;
>+}
>+
>+/*
>+ *
>+ */
>+static int irctl_close(struct inode *inode, struct file *file)
>+{
>+ struct irctl *ir = &irctls[MINOR(inode->i_rdev)];
>+
>+ dprintk(LOGHEAD "close called\n", ir->p.name, ir->p.minor);
>+
>+ /* if the plugin has a close function use it instead */
>+ if (ir->p.fops && ir->p.fops->release)
>+ return ir->p.fops->release(inode, file);
>+
>+ if (mutex_lock_interruptible(&plugin_lock))
>+ return -ERESTARTSYS;
>+
>+ --ir->open;
>+ if (ir->attached) {
>+ ir->p.set_use_dec(ir->p.data);
>+ module_put(ir->p.owner);
>+ } else {
>+ cleanup(ir);
>+ }
>+
>+ mutex_unlock(&plugin_lock);
>+
>+ return SUCCESS;
>+}
>+
>+/*
>+ *
>+ */
>+static unsigned int irctl_poll(struct file *file, poll_table *wait)
>+{
>+ struct irctl *ir = &irctls[MINOR(file->f_dentry->d_inode->i_rdev)];
>+ unsigned int ret;
>+
>+ dprintk(LOGHEAD "poll called\n", ir->p.name, ir->p.minor);
>+
>+ /* if the plugin has a poll function use it instead */
>+ if (ir->p.fops && ir->p.fops->poll)
>+ return ir->p.fops->poll(file, wait);
>+
>+ mutex_lock(&ir->buffer_lock);
>+ if (!ir->attached) {
>+ mutex_unlock(&ir->buffer_lock);
>+ return POLLERR;
>+ }
>+
>+ poll_wait(file, &ir->buf->wait_poll, wait);
>+
>+ dprintk(LOGHEAD "poll result = %s\n",
>+ ir->p.name, ir->p.minor,
>+ lirc_buffer_empty(ir->buf) ? "0" : "POLLIN|POLLRDNORM");
>+
>+ ret = lirc_buffer_empty(ir->buf) ? 0 : (POLLIN|POLLRDNORM);
>+
>+ mutex_unlock(&ir->buffer_lock);
>+ return ret;
>+}
>+
>+/*
>+ *
>+ */
>+static int irctl_ioctl(struct inode *inode, struct file *file,
>+ unsigned int cmd, unsigned long arg)
>+{
>+ unsigned long mode;
>+ int result;
>+ struct irctl *ir = &irctls[MINOR(inode->i_rdev)];
>+
>+ dprintk(LOGHEAD "ioctl called (0x%x)\n",
>+ ir->p.name, ir->p.minor, cmd);
>+
>+ /* if the plugin has a ioctl function use it instead */
>+ if (ir->p.fops && ir->p.fops->ioctl)
>+ return ir->p.fops->ioctl(inode, file, cmd, arg);
>+
>+ if (ir->p.minor == NOPLUG || !ir->attached) {
>+ dprintk(LOGHEAD "ioctl result = -ENODEV\n",
>+ ir->p.name, ir->p.minor);
>+ return -ENODEV;
>+ }
>+
>+ /* Give the plugin a chance to handle the ioctl */
>+ if (ir->p.ioctl) {
>+ result = ir->p.ioctl(inode, file, cmd, arg);
>+ if (result != -ENOIOCTLCMD)
>+ return result;
>+ }
>+ /* The plugin can't handle cmd */
>+ result = SUCCESS;
>+
>+ switch (cmd) {
>+ case LIRC_GET_FEATURES:
>+ result = put_user(ir->p.features, (unsigned long *)arg);
>+ break;
>+ case LIRC_GET_REC_MODE:
>+ if (!(ir->p.features&LIRC_CAN_REC_MASK))
>+ return -ENOSYS;
>+
>+ result = put_user(LIRC_REC2MODE
>+ (ir->p.features&LIRC_CAN_REC_MASK),
>+ (unsigned long *)arg);
>+ break;
>+ case LIRC_SET_REC_MODE:
>+ if (!(ir->p.features&LIRC_CAN_REC_MASK))
>+ return -ENOSYS;
>+
>+ result = get_user(mode, (unsigned long *)arg);
>+ if (!result && !(LIRC_MODE2REC(mode) & ir->p.features))
>+ result = -EINVAL;
>+ /*
>+ * FIXME: We should actually set the mode somehow but
>+ * for now, lirc_serial doesn't support mode changing either
>+ */
>+ break;
>+ case LIRC_GET_LENGTH:
>+ result = put_user((unsigned long)ir->p.code_length,
>+ (unsigned long *)arg);
>+ break;
>+ default:
>+ result = -ENOIOCTLCMD;
>+ }
>+
>+ dprintk(LOGHEAD "ioctl result = %d\n",
>+ ir->p.name, ir->p.minor, result);
>+
>+ return result;
>+}
>+
>+/*
>+ *
>+ */
>+static ssize_t irctl_read(struct file *file,
>+ char *buffer,
>+ size_t length,
>+ loff_t *ppos)
>+{
>+ struct irctl *ir = &irctls[MINOR(file->f_dentry->d_inode->i_rdev)];
>+ unsigned char buf[ir->buf->chunk_size];
>+ int ret = 0, written = 0;
>+ DECLARE_WAITQUEUE(wait, current);
>+
>+ dprintk(LOGHEAD "read called\n", ir->p.name, ir->p.minor);
>+
>+ /* if the plugin has a specific read function use it instead */
>+ if (ir->p.fops && ir->p.fops->read)
>+ return ir->p.fops->read(file, buffer, length, ppos);
>+
>+ if (mutex_lock_interruptible(&ir->buffer_lock))
>+ return -ERESTARTSYS;
>+ if (!ir->attached) {
>+ mutex_unlock(&ir->buffer_lock);
>+ return -ENODEV;
>+ }
>+
>+ if (length % ir->buf->chunk_size) {
>+ dprintk(LOGHEAD "read result = -EINVAL\n",
>+ ir->p.name, ir->p.minor);
>+ mutex_unlock(&ir->buffer_lock);
>+ return -EINVAL;
>+ }
>+
>+ /*
>+ * we add ourselves to the task queue before buffer check
>+ * to avoid losing scan code (in case when queue is awaken somewhere
>+ * beetwen while condition checking and scheduling)
>+ */
>+ add_wait_queue(&ir->buf->wait_poll, &wait);
>+ set_current_state(TASK_INTERRUPTIBLE);
>+
>+ /*
>+ * while we did't provide 'length' bytes, device is opened in blocking
>+ * mode and 'copy_to_user' is happy, wait for data.
>+ */
>+ while (written < length && ret == 0) {
>+ if (lirc_buffer_empty(ir->buf)) {
>+ /* According to the read(2) man page, 'written' can be
>+ * returned as less than 'length', instead of blocking
>+ * again, returning -EWOULDBLOCK, or returning
>+ * -ERESTARTSYS */
>+ if (written)
>+ break;
>+ if (file->f_flags & O_NONBLOCK) {
>+ ret = -EWOULDBLOCK;
>+ break;
>+ }
>+ if (signal_pending(current)) {
>+ ret = -ERESTARTSYS;
>+ break;
>+ }
>+ schedule();
>+ set_current_state(TASK_INTERRUPTIBLE);
>+ if (!ir->attached) {
>+ ret = -ENODEV;
>+ break;
>+ }
>+ } else {
>+ lirc_buffer_read_1(ir->buf, buf);
>+ ret = copy_to_user((void *)buffer+written, buf,
>+ ir->buf->chunk_size);
>+ written += ir->buf->chunk_size;
>+ }
>+ }
>+
>+ remove_wait_queue(&ir->buf->wait_poll, &wait);
>+ set_current_state(TASK_RUNNING);
>+ mutex_unlock(&ir->buffer_lock);
>+
>+ dprintk(LOGHEAD "read result = %s (%d)\n",
>+ ir->p.name, ir->p.minor, ret ? "-EFAULT" : "OK", ret);
>+
>+ return ret ? ret : written;
>+}
>+
>+
>+void *lirc_get_pdata(struct file *file)
>+{
>+ void *data = NULL;
>+
>+ if (file && file->f_dentry && file->f_dentry->d_inode &&
>+ file->f_dentry->d_inode->i_rdev) {
>+ struct irctl *ir;
>+ ir = &irctls[MINOR(file->f_dentry->d_inode->i_rdev)];
>+ data = ir->p.data;
>+ }
>+
>+ return data;
>+}
>+EXPORT_SYMBOL(lirc_get_pdata);
>+
>+
>+static ssize_t irctl_write(struct file *file, const char *buffer,
>+ size_t length, loff_t *ppos)
>+{
>+ struct irctl *ir = &irctls[MINOR(file->f_dentry->d_inode->i_rdev)];
>+
>+ dprintk(LOGHEAD "write called\n", ir->p.name, ir->p.minor);
>+
>+ /* if the plugin has a specific read function use it instead */
>+ if (ir->p.fops && ir->p.fops->write)
>+ return ir->p.fops->write(file, buffer, length, ppos);
>+
>+ if (!ir->attached)
>+ return -ENODEV;
>+
>+ return -EINVAL;
>+}
>+
>+
>+static struct file_operations fops = {
>+ .read = irctl_read,
>+ .write = irctl_write,
>+ .poll = irctl_poll,
>+ .ioctl = irctl_ioctl,
>+ .open = irctl_open,
>+ .release = irctl_close
>+};
>+
>+
>+static int lirc_dev_init(void)
>+{
>+ int i;
>+
>+ for (i = 0; i < MAX_IRCTL_DEVICES; ++i)
>+ init_irctl(&irctls[i]);
>+
>+ if (register_chrdev(IRCTL_DEV_MAJOR, IRCTL_DEV_NAME, &fops)) {
>+ printk(KERN_ERR "lirc_dev: register_chrdev failed\n");
>+ goto out;
>+ }
>+
>+ lirc_class = class_create(THIS_MODULE, "lirc");
>+ if (IS_ERR(lirc_class)) {
>+ printk(KERN_ERR "lirc_dev: class_create failed\n");
>+ goto out_unregister;
>+ }
>+
>+ printk(KERN_INFO "lirc_dev: IR Remote Control driver registered, "
>+ "major %d \n", IRCTL_DEV_MAJOR);
>+
>+ return SUCCESS;
>+
>+out_unregister:
>+ /* unregister_chrdev returns void now */
>+ unregister_chrdev(IRCTL_DEV_MAJOR, IRCTL_DEV_NAME);
>+out:
>+ return -1;
Ehm, the error code from register_chrdev() ?
>+}
>+
>+/* ---------------------------------------------------------------------- */
I hate those
>+
>+#ifdef MODULE
You don't need that one. subsys_initcall() does the right thing for you
allready.
>+
>+/*
>+ *
>+ */
>+int init_module(void)
>+{
>+ return lirc_dev_init();
>+}
>+
>+/*
>+ *
>+ */
>+void cleanup_module(void)
>+{
>+ /* unregister_chrdev returns void now */
>+ unregister_chrdev(IRCTL_DEV_MAJOR, IRCTL_DEV_NAME);
>+ class_destroy(lirc_class);
>+ dprintk("lirc_dev: module unloaded\n");
>+}
>+
>+MODULE_DESCRIPTION("LIRC base driver module");
>+MODULE_AUTHOR("Artur Lipowski");
>+MODULE_LICENSE("GPL");
>+MODULE_ALIAS_CHARDEV_MAJOR(IRCTL_DEV_MAJOR);
>+
>+module_param(debug, bool, 0644);
>+MODULE_PARM_DESC(debug, "Enable debugging messages");
>+
>+#else /* not a MODULE */
>+subsys_initcall(lirc_dev_init);
>+
>+#endif /* MODULE */
>+
>+/*
>+ * Overrides for Emacs so that we follow Linus's tabbing style.
>+ * ---------------------------------------------------------------------------
>+ * Local variables:
>+ * c-basic-offset: 8
>+ * End:
>+ */
Please don't enforce formating that way.
Regards,
Sebastian
--
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