Commit ae76e134 authored by Michal Nazarewicz's avatar Michal Nazarewicz Committed by Felipe Balbi

usb: f_fs: refactor ffs_epfile_io

Eliminate one of the return paths by using a ‘goto error_mutex’ and
rearrange some if-bodies which results in reduction of the indention level
and thus hopefully makes the function easier to read and reason about.
Signed-off-by: default avatarMichal Nazarewicz <mina86@mina86.com>
Signed-off-by: default avatarFelipe Balbi <balbi@kernel.org>
parent b3591f67
...@@ -684,6 +684,7 @@ static void ffs_epfile_async_io_complete(struct usb_ep *_ep, ...@@ -684,6 +684,7 @@ static void ffs_epfile_async_io_complete(struct usb_ep *_ep,
static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
{ {
struct ffs_epfile *epfile = file->private_data; struct ffs_epfile *epfile = file->private_data;
struct usb_request *req;
struct ffs_ep *ep; struct ffs_ep *ep;
char *data = NULL; char *data = NULL;
ssize_t ret, data_len = -EINVAL; ssize_t ret, data_len = -EINVAL;
...@@ -713,7 +714,8 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) ...@@ -713,7 +714,8 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
if (!halt) { if (!halt) {
/* /*
* if we _do_ wait above, the epfile->ffs->gadget might be NULL * if we _do_ wait above, the epfile->ffs->gadget might be NULL
* before the waiting completes, so do not assign to 'gadget' earlier * before the waiting completes, so do not assign to 'gadget'
* earlier
*/ */
struct usb_gadget *gadget = epfile->ffs->gadget; struct usb_gadget *gadget = epfile->ffs->gadget;
size_t copied; size_t copied;
...@@ -755,17 +757,12 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) ...@@ -755,17 +757,12 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
if (epfile->ep != ep) { if (epfile->ep != ep) {
/* In the meantime, endpoint got disabled or changed. */ /* In the meantime, endpoint got disabled or changed. */
ret = -ESHUTDOWN; ret = -ESHUTDOWN;
goto error_lock;
} else if (halt) { } else if (halt) {
/* Halt */ /* Halt */
if (likely(epfile->ep == ep) && !WARN_ON(!ep->ep)) if (likely(epfile->ep == ep) && !WARN_ON(!ep->ep))
usb_ep_set_halt(ep->ep); usb_ep_set_halt(ep->ep);
spin_unlock_irq(&epfile->ffs->eps_lock);
ret = -EBADMSG; ret = -EBADMSG;
} else { } else if (unlikely(data_len == -EINVAL)) {
/* Fire the request */
struct usb_request *req;
/* /*
* Sanity Check: even though data_len can't be used * Sanity Check: even though data_len can't be used
* uninitialized at the time I write this comment, some * uninitialized at the time I write this comment, some
...@@ -777,39 +774,9 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) ...@@ -777,39 +774,9 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
* For such reason, we're adding this redundant sanity check * For such reason, we're adding this redundant sanity check
* here. * here.
*/ */
if (unlikely(data_len == -EINVAL)) {
WARN(1, "%s: data_len == -EINVAL\n", __func__); WARN(1, "%s: data_len == -EINVAL\n", __func__);
ret = -EINVAL; ret = -EINVAL;
goto error_lock; } else if (!io_data->aio) {
}
if (io_data->aio) {
req = usb_ep_alloc_request(ep->ep, GFP_KERNEL);
if (unlikely(!req)) {
ret = -ENOMEM;
goto error_lock;
}
req->buf = data;
req->length = data_len;
io_data->buf = data;
io_data->ep = ep->ep;
io_data->req = req;
io_data->ffs = epfile->ffs;
req->context = io_data;
req->complete = ffs_epfile_async_io_complete;
ret = usb_ep_queue(ep->ep, req, GFP_ATOMIC);
if (unlikely(ret)) {
usb_ep_free_request(ep->ep, req);
goto error_lock;
}
ret = -EIOCBQUEUED;
spin_unlock_irq(&epfile->ffs->eps_lock);
} else {
DECLARE_COMPLETION_ONSTACK(done); DECLARE_COMPLETION_ONSTACK(done);
req = ep->req; req = ep->req;
...@@ -820,22 +787,22 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) ...@@ -820,22 +787,22 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
req->complete = ffs_epfile_io_complete; req->complete = ffs_epfile_io_complete;
ret = usb_ep_queue(ep->ep, req, GFP_ATOMIC); ret = usb_ep_queue(ep->ep, req, GFP_ATOMIC);
if (unlikely(ret < 0))
goto error_lock;
spin_unlock_irq(&epfile->ffs->eps_lock); spin_unlock_irq(&epfile->ffs->eps_lock);
if (unlikely(ret < 0)) { if (unlikely(wait_for_completion_interruptible(&done))) {
/* nop */
} else if (unlikely(
wait_for_completion_interruptible(&done))) {
ret = -EINTR; ret = -EINTR;
usb_ep_dequeue(ep->ep, req); usb_ep_dequeue(ep->ep, req);
} else { goto error_mutex;
}
/* /*
* XXX We may end up silently droping data * XXX We may end up silently droping data here. Since data_len
* here. Since data_len (i.e. req->length) may * (i.e. req->length) may be bigger than len (after being
* be bigger than len (after being rounded up * rounded up to maxpacketsize), we may end up with more data
* to maxpacketsize), we may end up with more * then user space has space for.
* data then user space has space for.
*/ */
ret = ep->status; ret = ep->status;
if (io_data->read && ret > 0) { if (io_data->read && ret > 0) {
...@@ -843,16 +810,38 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) ...@@ -843,16 +810,38 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
if (!ret) if (!ret)
ret = -EFAULT; ret = -EFAULT;
} }
} goto error_mutex;
kfree(data); } else if (!(req = usb_ep_alloc_request(ep->ep, GFP_KERNEL))) {
} ret = -ENOMEM;
} else {
req->buf = data;
req->length = data_len;
io_data->buf = data;
io_data->ep = ep->ep;
io_data->req = req;
io_data->ffs = epfile->ffs;
req->context = io_data;
req->complete = ffs_epfile_async_io_complete;
ret = usb_ep_queue(ep->ep, req, GFP_ATOMIC);
if (unlikely(ret)) {
usb_ep_free_request(ep->ep, req);
goto error_lock;
} }
mutex_unlock(&epfile->mutex); ret = -EIOCBQUEUED;
return ret; /*
* Do not kfree the buffer in this function. It will be freed
* by ffs_user_copy_worker.
*/
data = NULL;
}
error_lock: error_lock:
spin_unlock_irq(&epfile->ffs->eps_lock); spin_unlock_irq(&epfile->ffs->eps_lock);
error_mutex:
mutex_unlock(&epfile->mutex); mutex_unlock(&epfile->mutex);
error: error:
kfree(data); kfree(data);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment