site_archiver@lists.apple.com Delivered-To: darwin-kernel@lists.apple.com 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 ( 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 (Darwin-kernel@lists.apple.com) Help/Unsubscribe/Update your Subscription: http://lists.apple.com/mailman/options/darwin-kernel/site_archiver%40lists.a... Assuming this function is d_write in your cdevsw, it is read_write_fcn_t, which returns int, not "INT32". This email sent to site_archiver@lists.apple.com