Re: [PATCH] Bluetooth: qca: Use common error handling code in two functions

[Julia Lawall]

On Sun, 5 May 2024, Markus Elfring wrote:

> From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> Date: Sun, 5 May 2024 10:30:28 +0200
>
> Add a jump target so that the setting of an error code can be better reused
> at the end of these function implementations.
>
> This issue was transformed by using the Coccinelle software.

This reduces readability, and backwards jumps are rarely desirable.

julia

>
> Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/bluetooth/btqca.c | 35 +++++++++++++++++------------------
>  1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index cc61014ffbc9..1833aaa6d87b 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -108,10 +108,8 @@ static int qca_read_fw_build_info(struct hci_dev *hdev)
>  		return err;
>  	}
>
> -	if (skb->len < sizeof(*edl)) {
> -		err = -EILSEQ;
> -		goto out;
> -	}
> +	if (skb->len < sizeof(*edl))
> +		goto e_ilseq;
>
>  	edl = (struct edl_event_hdr *)(skb->data);
>
> @@ -123,17 +121,13 @@ static int qca_read_fw_build_info(struct hci_dev *hdev)
>  		goto out;
>  	}
>
> -	if (skb->len < sizeof(*edl) + 1) {
> -		err = -EILSEQ;
> -		goto out;
> -	}
> +	if (skb->len < sizeof(*edl) + 1)
> +		goto e_ilseq;
>
>  	build_lbl_len = edl->data[0];
>
> -	if (skb->len < sizeof(*edl) + 1 + build_lbl_len) {
> -		err = -EILSEQ;
> -		goto out;
> -	}
> +	if (skb->len < sizeof(*edl) + 1 + build_lbl_len)
> +		goto e_ilseq;
>
>  	build_label = kstrndup(&edl->data[1], build_lbl_len, GFP_KERNEL);
>  	if (!build_label)
> @@ -145,6 +139,10 @@ static int qca_read_fw_build_info(struct hci_dev *hdev)
>  out:
>  	kfree_skb(skb);
>  	return err;
> +
> +e_ilseq:
> +	err = -EILSEQ;
> +	goto out;
>  }
>
>  static int qca_send_patch_config_cmd(struct hci_dev *hdev)
> @@ -224,8 +222,7 @@ static int qca_read_fw_board_id(struct hci_dev *hdev, u16 *bid)
>  	edl = skb_pull_data(skb, sizeof(*edl));
>  	if (!edl) {
>  		bt_dev_err(hdev, "QCA read board ID with no header");
> -		err = -EILSEQ;
> -		goto out;
> +		goto e_ilseq;
>  	}
>
>  	if (edl->cresp != EDL_CMD_REQ_RES_EVT ||
> @@ -235,10 +232,8 @@ static int qca_read_fw_board_id(struct hci_dev *hdev, u16 *bid)
>  		goto out;
>  	}
>
> -	if (skb->len < 3) {
> -		err = -EILSEQ;
> -		goto out;
> -	}
> +	if (skb->len < 3)
> +		goto e_ilseq;
>
>  	*bid = (edl->data[1] << 8) + edl->data[2];
>  	bt_dev_dbg(hdev, "%s: bid = %x", __func__, *bid);
> @@ -246,6 +241,10 @@ static int qca_read_fw_board_id(struct hci_dev *hdev, u16 *bid)
>  out:
>  	kfree_skb(skb);
>  	return err;
> +
> +e_ilseq:
> +	err = -EILSEQ;
> +	goto out;
>  }
>
>  int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
> --
> 2.44.0
>
>
>