Re: [PATCH] nfc: pn533: remove redundant assignment

[Jakub Kicinski]

On Fri,  9 Apr 2021 19:50:09 +0800 samirweng1979 wrote:
> From: wengjianfeng <wengjianf...@yulong.com>
> 
> In many places,first assign a value to a variable and then return
> the variable. which is redundant, we should directly return the value.
> in pn533_rf_field funciton,return statement in the if statement is
> redundant, we just delete it.
> 
> Signed-off-by: wengjianfeng <wengjianf...@yulong.com>

Thank you for the changes, please see comments below.

> diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
> index f1469ac..61ab4c0 100644
> --- a/drivers/nfc/pn533/pn533.c
> +++ b/drivers/nfc/pn533/pn533.c
> @@ -489,12 +489,8 @@ static int pn533_send_data_async(struct pn533 *dev, u8 
> cmd_code,
>                                pn533_send_async_complete_t complete_cb,
>                                void *complete_cb_context)
>  {
> -     int rc;
> -
> -     rc = __pn533_send_async(dev, cmd_code, req, complete_cb,
> +     return __pn533_send_async(dev, cmd_code, req, complete_cb,
>                               complete_cb_context);

Please realign the continuation line so it starts after the opening
bracket.

> -
> -     return rc;
>  }
>  
>  static int pn533_send_cmd_async(struct pn533 *dev, u8 cmd_code,
> @@ -502,12 +498,8 @@ static int pn533_send_cmd_async(struct pn533 *dev, u8 
> cmd_code,
>                               pn533_send_async_complete_t complete_cb,
>                               void *complete_cb_context)
>  {
> -     int rc;
> -
> -     rc = __pn533_send_async(dev, cmd_code, req, complete_cb,
> +     return __pn533_send_async(dev, cmd_code, req, complete_cb,
>                               complete_cb_context);

Same here.

> -     return rc;
>  }
>  
>  /*
> @@ -2614,7 +2606,6 @@ static int pn533_rf_field(struct nfc_dev *nfc_dev, u8 
> rf)
>                                    (u8 *)&rf_field, 1);
>       if (rc) {
>               nfc_err(dev->dev, "Error on setting RF field\n");
> -             return rc;
>       }
>  
>       return rc;

In case some code is added between the check and the return statement
it'd be better to fix this by replacing the second return rc with
return 0:

        if (rc) {
                nfc_err(...);
                return rc;
        }

        return 0;

> diff --git a/drivers/nfc/pn533/uart.c b/drivers/nfc/pn533/uart.c
> index a0665d8..6465348 100644
> --- a/drivers/nfc/pn533/uart.c
> +++ b/drivers/nfc/pn533/uart.c
> @@ -239,9 +239,8 @@ static int pn532_uart_probe(struct serdev_device *serdev)
>  {
>       struct pn532_uart_phy *pn532;
>       struct pn533 *priv;
> -     int err;
> +     int err = -ENOMEM;
>  
> -     err = -ENOMEM;
>       pn532 = kzalloc(sizeof(*pn532), GFP_KERNEL);

IMO having the assignment before the call is more readable, please
leave as is.

>       if (!pn532)
>               goto err_exit;