On Wednesday 30 January 2013 12:37:25 Kurachkin Michail wrote:
> Hi Greg,
> 
> I followed your recommendations and created a diff using Linux 3.8-rc5 
> sources. Please review it and give your comments.

Part #3

+/*
+ * method "open" for character device
+ */
+static int slic_chr_open(struct inode *inode, struct file *file)
+{
+       struct slic_chr_dev *chr_dev;
+       struct si3226x_slic *slic;
+       struct si3226x_line *line;
+       struct tdm_device *tdm_dev;
+       struct tdm_voice_channel *ch;
+       int status = -ENXIO;
+
+       mutex_lock(&slic_chr_dev_lock);
+
+       list_for_each_entry(chr_dev, &slic_chr_dev_list, list) {
+               switch (chr_dev->type) {
+               case SLIC_CHR_DEV:
+                       slic = dev_get_drvdata(chr_dev->dev);
+
+                       if (slic->devt != inode->i_rdev)
+                               continue;;
+
+                       if (slic->file_opened) {
+                               status = -EBUSY;
+                               goto out;
+                       }
+
+                       slic->file_opened = 1;
+                       status = 0;
+                       break;
+
+               case LINE_CHR_DEV:
+                       line = dev_get_drvdata(chr_dev->dev);
+                       tdm_dev = line->tdm_dev;
+
+                       if (line->devt != inode->i_rdev)
+                               continue;
+
+                       if (line->file_opened) {
+                               status = -EBUSY;
+                               goto out;
+                       }
+
+                       line->audio_buffer_size = 
tdm_get_voice_block_size(tdm_dev);
+
+                       line->rx_buffer = kzalloc(line->audio_buffer_size, 
GFP_KERNEL);
+                       if (!line->rx_buffer) {
+                               status = -ENOMEM;
+                               goto out;
+                       }
+
+                       line->tx_buffer = kzalloc(line->audio_buffer_size, 
GFP_KERNEL);
+                       if (!line->tx_buffer) {
+                               status = -ENOMEM;

memory leak of line->rx_buffer

+                               goto out;
+                       }
+
+                       /* store pointer to transmit wait_queue_head_t for use 
in poll() */
+                       ch = tdm_dev->ch;
+                       line->tx_wait_queue = &ch->tx_queue;
+                       line->rx_wait_queue = &ch->rx_queue;
+                       line->file_opened = 1;
+                       status = 0;
+
+                       break;
+               }
+
+               file->private_data = chr_dev;
+               status = 0;
+               goto out;

a bit pointless

+       }
+
+out:
+       mutex_unlock(&slic_chr_dev_lock);
+       return status;
+}

+/*
+ * method "ioctl" for character device
+ */
+static long
+slic_chr_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+       struct slic_chr_dev *chr_dev = file->private_data;
+       struct si3226x_slic *slic;
+       struct si3226x_line *line;
+       int status = -ENXIO;
+
+       if (_IOC_TYPE(cmd) != SI3226X_IOC_MAGIC)
+               return -ENOTTY;
+
+       mutex_lock(&slic_chr_dev_lock);
+
+       if (!file->private_data) {
+               status = -EPERM;

-EPERM?

+               goto out;
+       }
+
+       switch(chr_dev->type) {
+       case SLIC_CHR_DEV:
+               slic = dev_get_drvdata(chr_dev->dev);
+               if (!slic->file_opened) {
+                       status = -ENODEV;
+                       goto out;
+               }
+
+               status = slic_control(slic, cmd, arg);
+               break;
+
+       case LINE_CHR_DEV:
+               line = dev_get_drvdata(chr_dev->dev);
+               if (!line->file_opened) {
+                       status = -ENODEV;
+                       goto out;
+               }
+
+               status = line_control(line, cmd, arg);
+               break;
+       }
+
+out:
+       mutex_unlock(&slic_chr_dev_lock);
+       return status;
+}


+/*
+ * method "read" for character device
+ */
+static ssize_t
+slic_chr_read(struct file *file, char *buff, size_t count, loff_t *offp)
+{
+       struct slic_chr_dev *chr_dev = file->private_data;
+       struct si3226x_line *line;
+       struct tdm_device *tdm_dev;
+       int rc;
+
+       if (!file->private_data)
+               return -EPERM;
+
+       if(chr_dev->type != LINE_CHR_DEV)
+               return -EPERM;
+
+       mutex_lock(&slic_chr_dev_lock);
+
+       line = dev_get_drvdata(chr_dev->dev);
+       tdm_dev = line->tdm_dev;
+
+       if (count != line->audio_buffer_size) {
+               rc = -ENODATA;
+               goto out;
+       }
+
+       rc = tdm_recv(tdm_dev, line->rx_buffer);
+       if (rc) {
+               rc = -EFAULT;

EFAULT is usually not what you return upon a read error.

+               goto out;
+       }
+
+       rc = copy_to_user(buff, line->rx_buffer, count);
+       if (rc) {
+               rc = -EFAULT;
+               goto out;
+       }
+
+       rc = count;
+out:
+       mutex_unlock(&slic_chr_dev_lock);
+       return rc;
+}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to