Re: [bug report] pinctrl: mediatek: Add EINT support for multiple addresses

[Dan Carpenter <dan.carpenter@linaro.org>]

On Tue, Mar 25, 2025 at 08:45:13AM +0300, Dan Carpenter wrote:
> Hello Hao Chang,
> 
> Commit 3ef9f710efcb ("pinctrl: mediatek: Add EINT support for
> multiple addresses") from Mar 22, 2025 (linux-next), leads to the
> following Smatch static checker warning:
> 
> 	drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c:384 mtk_build_eint()
> 	warn: error code type promoted to positive: 'count_reg_names'
> 
> drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
>     374                 return 0;
>     375 
>     376         if (!of_property_read_bool(np, "interrupt-controller"))
>     377                 return -ENODEV;
>     378 
>     379         hw->eint = devm_kzalloc(hw->dev, sizeof(*hw->eint), GFP_KERNEL);
>     380         if (!hw->eint)
>     381                 return -ENOMEM;
>     382 
>     383         count_reg_names = of_property_count_strings(np, "reg-names");
> --> 384         if (count_reg_names < hw->soc->nbase_names)
> 
> count_reg_names is type int but hw->soc->nbase_names is unsigned int.  So
> if of_property_count_strings() returns a negative error code then it's
> type promoted as a high positive unsigned int value and treated as success.
> 
>     385                 return -EINVAL;
>     386 
>     387         hw->eint->nbase = count_reg_names - hw->soc->nbase_names;
> 
> hw->eint->nbase is a u8 so the negative value is truncated to a low positive
> value.
> 
>     388         hw->eint->base = devm_kmalloc_array(&pdev->dev, hw->eint->nbase,
>     389                                             sizeof(*hw->eint->base), GFP_KERNEL | __GFP_ZERO);
> 
> This allocation will always succeed.
> 
> regards,
> dan carpenter
> 
>     390         if (!hw->eint->base) {
>     391                 ret = -ENOMEM;
>     392                 goto err_free_base;
>     393         }
>     394 
>     395         for (i = hw->soc->nbase_names, j = 0; i < count_reg_names; i++, j++) {
>     396                 hw->eint->base[j] = of_iomap(np, i);
>     397                 if (IS_ERR(hw->eint->base[j])) {

Oh, also of_iomap() returns NULL on errors, not error pointers.

regards,
dan carpenter

>     398                         ret = PTR_ERR(hw->eint->base[j]);
>     399                         goto err_free_eint;
>     400                 }
>     401         }
>     402