Re: [PATCH] hv: utils: fix coding style

From: JesÃs Castro
Date: Mon Mar 11 2019 - 19:22:10 EST


> > Checkpatch script is showing a coding style error and is showing:
> >
> > ERROR: else should follow close brace '}'
> > + }
> > + else if (hvt->mode == HVUTIL_TRANSPORT_NETLINK) {
> >
> > This commit fixes the coding style, not a functional change.
> []
> > diff --git a/drivers/hv/hv_utils_transport.c b/drivers/hv/hv_utils_transport.c
> []
> > @@ -139,8 +139,7 @@ static int hvt_op_open(struct inode *inode, struct file *file)
> > * device gets released.
> > */
> > hvt->mode = HVUTIL_TRANSPORT_CHARDEV;
> > - }
> > - else if (hvt->mode == HVUTIL_TRANSPORT_NETLINK) {
> > + } else if (hvt->mode == HVUTIL_TRANSPORT_NETLINK) {
> > /*
> > * We're switching from netlink communication to using char
> > * device. Issue the reset first.
>
> Rather than merely shutting up checkpatch warnings,
> please try to improve the code for humans to read.
>
> This block is probably better as a switch/case and
> the separate bool issue_reset automatic seems
> unnecessary as it's only used in one case.
>

I'll follow your suggestion and send the v2.

Thanks for your advice to look beyond the scripts, I'll take it into
account for the next patches.

> Perhaps:
> ---
> drivers/hv/hv_utils_transport.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hv/hv_utils_transport.c b/drivers/hv/hv_utils_transport.c
> index 832777527936..6b27dd7be9bb 100644
> --- a/drivers/hv/hv_utils_transport.c
> +++ b/drivers/hv/hv_utils_transport.c
> @@ -125,35 +125,35 @@ static int hvt_op_open(struct inode *inode, struct file *file)
> {
> struct hvutil_transport *hvt;
> int ret = 0;
> - bool issue_reset = false;
>
> hvt = container_of(file->f_op, struct hvutil_transport, fops);
>
> mutex_lock(&hvt->lock);
>
> - if (hvt->mode == HVUTIL_TRANSPORT_DESTROY) {
> - ret = -EBADF;
> - } else if (hvt->mode == HVUTIL_TRANSPORT_INIT) {
> + switch (hvt->mode) {
> + case HVUTIL_TRANSPORT_INIT:
> /*
> * Switching to CHARDEV mode. We switch bach to INIT when
> * device gets released.
> */
> hvt->mode = HVUTIL_TRANSPORT_CHARDEV;
> - }
> - else if (hvt->mode == HVUTIL_TRANSPORT_NETLINK) {
> + break;
> + case HVUTIL_TRANSPORT_NETLINK:
> /*
> * We're switching from netlink communication to using char
> * device. Issue the reset first.
> */
> - issue_reset = true;
> hvt->mode = HVUTIL_TRANSPORT_CHARDEV;
> - } else {
> + hvt_reset(hvt);
> + break;
> + case HVUTIL_TRANSPORT_DESTROY:
> + ret = -EBADF;
> + break;
> + default:
> ret = -EBUSY;
> + break;
> }
>
> - if (issue_reset)
> - hvt_reset(hvt);
> -
> mutex_unlock(&hvt->lock);
>
> return ret;
>
>