From mboxrd@z Thu Jan 1 00:00:00 1970 From: LABBE Corentin Subject: Re: [PATCH v8] Input: Add userio module Date: Fri, 9 Oct 2015 16:52:44 +0200 Message-ID: <20151009145244.GB20559@Red> References: <1444401053-6686-1-git-send-email-cpaul@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1444401053-6686-1-git-send-email-cpaul@redhat.com> Sender: linux-doc-owner@vger.kernel.org To: cpaul@redhat.com Cc: David Herrmann , Dmitry Torokhov , linux-kernel , Linux API , Andrew Morton , Mauro Carvalho Chehab , Greg KH , Arnd Bergmann , Joe Perches , Jiri Slaby , Vishnu Patekar , Sebastian Ott , Benjamin Tissoires , Hans de Goede , linux-doc@vger.kernel.org List-Id: linux-api@vger.kernel.org On Fri, Oct 09, 2015 at 10:30:53AM -0400, cpaul@redhat.com wrote: > From: Stephen Chandler Paul > + > +static ssize_t userio_char_read(struct file *file, char __user *user_buffer, > + size_t count, loff_t *ppos) > +{ > + struct userio_device *userio = file->private_data; > + int ret; > + size_t nonwrap_len, copylen; > + unsigned char buf[USERIO_BUFSIZE]; > + unsigned long flags; > + > + if (!count) > + return 0; > + > + /* > + * By the time we get here, the data that was waiting might have been > + * taken by another thread. Grab the mutex and check if there's still > + * any data waiting, otherwise repeat this process until we have data > + * (unless the file descriptor is non-blocking of course) > + */ > + for (;;) { > + spin_lock_irqsave(&userio->buf_lock, flags); > + > + if (userio->head != userio->tail) > + break; > + > + spin_unlock_irqrestore(&userio->buf_lock, flags); > + > + if (file->f_flags & O_NONBLOCK) > + return -EAGAIN; > + > + ret = wait_event_interruptible(userio->waitq, > + userio->head != userio->tail); > + if (ret) > + return ret; > + } > + > + nonwrap_len = CIRC_CNT_TO_END(userio->head, userio->tail, > + USERIO_BUFSIZE); > + copylen = min(nonwrap_len, count); > + memcpy(buf, &userio->buf[userio->tail], copylen); > + > + userio->tail = (userio->tail + copylen) % USERIO_BUFSIZE; > + > + spin_unlock_irqrestore(&userio->buf_lock, flags); > + > + if (copy_to_user(user_buffer, buf, copylen)) > + return -EFAULT; > + > + return copylen; > +} Hello I think you could simplify by just return copy_to_user() since it return copylen in case of success. Regards