Re: [PATCH] New: Omnikey CardMan 4040 PCMCIA Driver

[Alexey Dobriyan]

On Sun, Sep 04, 2005 at 12:12:18PM +0200, Harald Welte wrote:
> Below you can find a driver for the Omnikey CardMan 4040 PCMCIA
> Smartcard Reader.  

> --- /dev/null
> +++ b/drivers/char/pcmcia/cm4040_cs.c

> +#include <linux/config.h>

Not needed.

> +static volatile char *version =

Can we lose all volatile and register keywords?

> +typedef struct reader_dev_t {
> +	dev_link_t		link;		
> +	dev_node_t		node;		
> +	wait_queue_head_t	devq;	
> +
> +	wait_queue_head_t	poll_wait;
> +	wait_queue_head_t	read_wait;
> +	wait_queue_head_t	write_wait;
> +
> +	unsigned int 	  	buffer_status;
> +                                
> +	unsigned int      	fTimerExpired;
> +	struct timer_list	timer;
> +	unsigned long     	timeout;
> +	unsigned char     	sBuf[READ_WRITE_BUFFER_SIZE];
> +	unsigned char     	rBuf[READ_WRITE_BUFFER_SIZE];
> +	struct task_struct 	*owner;	
> +} reader_dev_t;

And typedefs too.

	struct reader_dev {
	
	};

> +static ssize_t cmx_read(struct file *filp,char *buf,size_t count,loff_t *ppos)

char __user *buf

> +	ulBytesToRead = 5 + 
> +			(0x000000FF&((char)dev->rBuf[1])) + 
> +			(0x0000FF00&((char)dev->rBuf[2] << 8)) + 
> +			(0x00FF0000&((char)dev->rBuf[3] << 16)) + 
> +			(0xFF000000&((char)dev->rBuf[4] << 24));

	ulBytesToRead = 5 + le32_to_cpu(*(__le32 *)&dev->rBuf[1]);

> +	ulMin = (count < (ulBytesToRead+5))?count:(ulBytesToRead+5);

	ulMin = min(count, ulBytesToRead + 5);

> +	copy_to_user(buf, dev->rBuf, ulMin);

Can fail.

> +static ssize_t cmx_write(struct file *filp,const char *buf,size_t count,

const char __user *buf

> +			 loff_t *ppos)

> +	copy_from_user(dev->sBuf, buf, uiBytesToWrite);

Can fail.

> +static int cmx_ioctl(struct inode *inode,struct file *filp,unsigned int cmd,
> +			unsigned long arg)
> +{
> +	dev_link_t *link;
> +	int rc, size;
> +
> +	link=dev_table[MINOR(inode->i_rdev)];
> +	if (!(DEV_OK(link))) {
> +		DEBUG(4, "DEV_OK false\n");
> +		return -ENODEV;
> +	}
> +	if (_IOC_TYPE(cmd)!=CM_IOC_MAGIC) {
> +		DEBUG(4,"ioctype mismatch\n");
> +		return -EINVAL;
> +	}
> +	if (_IOC_NR(cmd)>CM_IOC_MAXNR) {
> +		DEBUG(4,"iocnr mismatch\n");
> +		return -EINVAL;
> +	} 
> +	size = _IOC_SIZE(cmd);
> +	rc = 0;
> +	DEBUG(4,"iocdir=%.4x iocr=%.4x iocw=%.4x iocsize=%d cmd=%.4x\n",
> +		_IOC_DIR(cmd),_IOC_READ,_IOC_WRITE,size,cmd);
> +
> +	if (_IOC_DIR(cmd)&_IOC_READ) {
> +		if (!access_ok(VERIFY_WRITE, (void *)arg, size))
> +			return -EFAULT;
> +	}
> +	if (_IOC_DIR(cmd)&_IOC_WRITE) {
> +		if (!access_ok(VERIFY_READ, (void *)arg, size))
> +			return -EFAULT;
> +	}
> +
> +	return rc;
> +}

Whoo, empty ioctl handler.

> +static void reader_release(u_long arg)

> +	link = (dev_link_t *)arg;

You do

	reader_release((unsigned long)link);

somewhere above and below.

> +static void reader_detach_by_devno(int devno,dev_link_t *link)

> +		reader_release((u_long)link);

Like this.

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