Re: Problems implementing write function of a character device
Re: Problems implementing write function of a character device
- Subject: Re: Problems implementing write function of a character device
- From: Michael Smith <email@hidden>
- Date: Wed, 13 Sep 2006 08:48:48 -0700
On Sep 13, 2006, at 1:54 AM, sanjay yaragatti wrote:
I am implementing write func for a character device.
I am using the accessor methods uio_curriovlen(uio)
and uio_curriovbase(uio) provided in the <sys/uio.h>
file to get the length value of the current iovec
associated with the given uio and the base address of
the current iovec associated with the given uio
respectively.However, both the calls are returning
zero as the return value.Hence unable to copy the data
from userspace to kernel space.Even tried using
uiomove() inplace of copyin() but doesnt work. I have
attached the code for reference.. Please let me know
if something wrong in the code.
INT32 device_write
Assuming this function is d_write in your cdevsw, it is
read_write_fcn_t, which
returns int, not "INT32".
(
dev_t device, struct uio *uio, int ioflag
)
{
INT ret = SUCCESS;
INT is spelled "int".
PUCHAR tmp_buff = NULL;
What is "PUCHAR"? Do you mean "unsigned char *"? If so, use it.
UINT32 minor_num;
UINT32 is spelled "uint32_t".
user_size_t buff_len = 0, buff_base = 0;
/* Get the minor number */
minor_num = minor(device);
buff_len = uio_curriovlen(uio);
buff_base = uio_curriovbase(uio);
uio_curriovbase() returns user_addr_t not user_size_t.
IOLog ("uio_curriovlen is %d & uio_curriovbase
is %d\n ",buff_len,buff_base);
tmp_buff = (PUCHAR) mem_alloc(buff_len);
You should never, ever allocate memory on your hot path.
For something like this, you would probably be fine with
a buffer on the stack.
if (NULL == tmp_buff)
{
IOLog("%s: Buffer is NULL\n", __FUNCTION__);
return FAILURE;
}
if(ret = copyin(buff_base, tmp_buff,buff_len)!= 0)
Why are you using copyin? Why aren't you updating the uio?
Why aren't you checking whether the buffer is already in kernel
space? How do you plan to deal with writev() calls that have
more than one segment in the iovec?
This is why you should use uiomove.
{
mem_free(tmp_buff,buff_len);
IOlog("%s: Copy from user failed\n",
__FUNCTION__);
return FAILURE;
}
*(tmp_buff+ buff_len ) = 0;
}
And why don't you free the buffer in the success case? You're
going to leak kernel memory here.
Try something like
#define WRITE_CHUNK_SIZE 128
int
device_write(dev_t *dev, struct uio *uio, int ioflag)
{
char stk_buf[WRITE_CHUNK_SIZE];
int buf_len;
/* check direction */
if (uio_rw(uio) != UIO_WRITE)
return(EINVAL);
/* loop bringing in chunks of data */
for (;;) {
/* check UIO for more data */
buf_len = min(uio_resid(uio), WRITE_CHUNK_SIZE);
if (buf_len == 0)
break;
/* bring in a chunk */
uiomove(stk_buf, buf_len, uio);
/* emit chunk */
driver_emit_bytes(stk_buf, buf_len);
}
return(0);
}
Note that this as it stands is not very efficient; depending on your
device you may want to bring in much larger chunks, or if you have
a buffer pool for outbound data you might want to grab buffers in
the uiomove loop and move into them rather than into a temporary
stack buffer.
If your device is fairly slow, however, this will work fine. You
would not want to increase WRITE_CHUNK_SIZE to much more
than 1KB or so, depending on how much stack you use elsewhere
in your driver. With buffers that large you're looking at a higher-
performance design though and mapping directly will start to
be more attractive.
= Mike
_______________________________________________
Do not post admin requests to the list. They will be ignored.
Darwin-kernel mailing list (email@hidden)
Help/Unsubscribe/Update your Subscription:
This email sent to email@hidden