Hi,
Static analysis with Coverity has detected a potential issue with the
following commit:
commit 39c1a9728f938c7255ce507c8d56b73e8a4ebddf
Author: Ihab Zhaika <[email protected]>
Date: Fri Nov 15 09:28:11 2019 +0200
iwlwifi: refactor the SAR tables from mvm to acpi
in function iwl_sar_get_ewrd_table() we have an array index pos being
initialized to 3 and then incremented each time a loop iterates:
for (i = 0; i < n_profiles; i++) {
/* the tables start at element 3 */
int pos = 3;
/* The EWRD profiles officially go from 2 to 4, but we
* save them in sar_profiles[1-3] (because we don't
* have profile 0). So in the array we start from 1.
*/
ret = iwl_sar_set_profile(&wifi_pkg->package.elements[pos],
&fwrt->sar_profiles[i + 1],
enabled);
if (ret < 0)
break;
/* go to the next table */
pos += ACPI_SAR_TABLE_SIZE;
}
So, each iteration is always accessing package.elements[3]. I'm not sure
if that is intentional. If it is, then the increment of pos is not
required. Either way, it's not clear what the original intention is.
Colin
On Fri, 2019-12-20 at 00:02 +0000, Colin Ian King wrote:
> Hi,
>
> Static analysis with Coverity has detected a potential issue with the
> following commit:
>
> commit 39c1a9728f938c7255ce507c8d56b73e8a4ebddf
> Author: Ihab Zhaika <[email protected]>
> Date: Fri Nov 15 09:28:11 2019 +0200
>
> iwlwifi: refactor the SAR tables from mvm to acpi
>
>
> in function iwl_sar_get_ewrd_table() we have an array index pos being
> initialized to 3 and then incremented each time a loop iterates:
>
> for (i = 0; i < n_profiles; i++) {
> /* the tables start at element 3 */
> int pos = 3;
>
> /* The EWRD profiles officially go from 2 to 4, but we
> * save them in sar_profiles[1-3] (because we don't
> * have profile 0). So in the array we start from 1.
> */
> ret = iwl_sar_set_profile(&wifi_pkg->package.elements[pos],
> &fwrt->sar_profiles[i + 1],
> enabled);
> if (ret < 0)
> break;
>
> /* go to the next table */
> pos += ACPI_SAR_TABLE_SIZE;
> }
>
> So, each iteration is always accessing package.elements[3]. I'm not sure
> if that is intentional. If it is, then the increment of pos is not
> required. Either way, it's not clear what the original intention is.
Good catch, thanks! We'll fix it.
--
Cheers,
Luca.