Re: [PATCH][next] Bluetooth: Use struct_size() helper

[Joe Perches <joe@perches.com>]

On Thu, 2019-03-28 at 12:30 -0500, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
> 
> struct foo {
>     int stuff;
>     struct boo entry[];
> };
> 
> size = sizeof(struct foo) + count * sizeof(struct boo);
> 
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
> 
> size = struct_size(instance, entry, count);
[]
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
[]
> @@ -2301,8 +2301,7 @@ static int load_link_keys(struct sock *sk, struct hci_dev *hdev, void *data,
>  				       MGMT_STATUS_INVALID_PARAMS);
>  	}
>  
> -	expected_len = sizeof(*cp) + key_count *
> -					sizeof(struct mgmt_link_key_info);
> +	expected_len = struct_size(cp, keys, key_count);
>  	if (expected_len != len) {
>  		bt_dev_err(hdev, "load_link_keys: expected %u bytes, got %u bytes",
>  			   expected_len, len);
> @@ -5030,7 +5029,7 @@ static int load_irks(struct sock *sk, struct hci_dev *hdev, void *cp_data,
>  				       MGMT_STATUS_INVALID_PARAMS);
>  	}
>  
> -	expected_len = sizeof(*cp) + irk_count * sizeof(struct mgmt_irk_info);
> +	expected_len = struct_size(cp, irks, irk_count);
>  	if (expected_len != len) {
>  		bt_dev_err(hdev, "load_irks: expected %u bytes, got %u bytes",
>  			   expected_len, len);
> @@ -5112,8 +5111,7 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
>  				       MGMT_STATUS_INVALID_PARAMS);
>  	}
>  
> -	expected_len = sizeof(*cp) + key_count *
> -					sizeof(struct mgmt_ltk_info); 
> +	expected_len = struct_size(cp, keys, key_count);
>  	if (expected_len != len) {
>  		bt_dev_err(hdev, "load_keys: expected %u bytes, got %u bytes",
>  			   expected_len, len);
> @@ -5847,8 +5845,7 @@ static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
>  				       MGMT_STATUS_INVALID_PARAMS);
>  	}
>  
> -	expected_len = sizeof(*cp) + param_count *
> -					sizeof(struct mgmt_conn_param);
> +	expected_len = struct_size(cp, params, param_count);
>  	if (expected_len != len) {
>  		bt_dev_err(hdev, "load_conn_param: expected %u bytes, got %u bytes",
>  			   expected_len, len);

Maybe add a helper function for all of these.
Perhaps something like:

static bool verify_len(size_t actual, size_t expected)
{
	if (actual == expected)
		return true;
	bt_dev_err("%ps: expected %zu bytes, got %zu bytes",
		   __builtin_return_address(1), expected, actual);
	return false;
}

So the last block above might be

	if (!verify_len(len, struct_size(cp, params, param_count),))
		return mgmt_cmd_status(sk, hdev->id,  MGMT_OP_LOAD_CONN_PARAM,
				       MGMT_STATUS_INVALID_PARAMS);

etc...