Messages in this thread | | | Date | Mon, 25 Apr 2005 14:23:51 -0700 | From | Greg KH <> | Subject | Re: [PATCH 5/7] dlm: device interface |
| |
On Mon, Apr 25, 2005 at 11:13:03PM +0800, David Teigland wrote: > +#ifndef __KERNEL > +#define __user > +#endif
What is this for?
> +/* struct passed to the lock write */ > +struct dlm_lock_params { > + uint8_t mode; > + uint16_t flags; > + uint32_t lkid; > + uint32_t parent;
These are crossing the userspace/kernelspace boundry, please mark them __u8, __u16, and __u32.
> + struct dlm_range range; > + uint8_t namelen; > + void __user *castparam; > + void __user *castaddr; > + void __user *bastparam; > + void __user *bastaddr; > + struct dlm_lksb __user *lksb; > + char lvb[DLM_LVB_LEN]; > + char name[1];
Mix of tabs and spaces.
> +/* > + * ioctls to create/remove lockspaces, and check how many > + * outstanding ASTs there are against a particular LS. > + */
Your comment is wrong, based on the code.
> +static int dlm_ioctl(struct inode *inode, struct file *file, > + uint command, ulong u) > +{ > + struct file_info *fi = file->private_data; > + int status = -EINVAL; > + int count; > + struct list_head *tmp_list; > + > + switch (command) { > + > + /* Are there any ASTs for us to read? > + * Warning, this returns the number of messages (ASTs) > + * in the queue, NOT the number of bytes to read > + */ > + case FIONREAD: > + count = 0; > + spin_lock(&fi->fi_ast_lock); > + list_for_each(tmp_list, &fi->fi_ast_list) > + count++; > + spin_unlock(&fi->fi_ast_lock); > + status = put_user(count, (int *)u); > + break;
Is this really needed?
thanks,
greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |