[bug report] ALSA: hda/tas2781: Add tas2781 hda SPI driver

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

Hello Baojun Xu,

Commit bb5f86ea50ff ("ALSA: hda/tas2781: Add tas2781 hda SPI driver")
from Dec 16, 2024 (linux-next), leads to the following Smatch static
checker warning:

	sound/pci/hda/tas2781_hda_spi.c:1107 tas2781_hda_spi_probe()
	warn: missing error code here? 'devm_kzalloc()' failed. 'ret' = '0'

sound/pci/hda/tas2781_hda_spi.c
    1092 static int tas2781_hda_spi_probe(struct spi_device *spi)
    1093 {
    1094         struct tasdevice_priv *tas_priv;
    1095         struct tas2781_hda *tas_hda;
    1096         const char *device_name;
    1097         int ret = 0;
    1098 
    1099         tas_hda = devm_kzalloc(&spi->dev, sizeof(*tas_hda), GFP_KERNEL);
    1100         if (!tas_hda)
    1101                 return -ENOMEM;
    1102 
    1103         spi->max_speed_hz = TAS2781_SPI_MAX_FREQ;
    1104 
    1105         tas_priv = devm_kzalloc(&spi->dev, sizeof(*tas_priv), GFP_KERNEL);
    1106         if (!tas_priv)
--> 1107                 goto err;

No error code.

    1108         tas_priv->dev = &spi->dev;
    1109         tas_hda->priv = tas_priv;
    1110         tas_priv->regmap = devm_regmap_init_spi(spi, &tasdevice_regmap);
    1111         if (IS_ERR(tas_priv->regmap)) {
    1112                 ret = PTR_ERR(tas_priv->regmap);
    1113                 dev_err(tas_priv->dev, "Failed to allocate regmap: %d\n",
    1114                         ret);
    1115                 goto err;
    1116         }
    1117         if (strstr(dev_name(&spi->dev), "TXNW2781")) {
    1118                 device_name = "TXNW2781";
    1119                 tas_priv->save_calibration = tas2781_save_calibration;
    1120                 tas_priv->apply_calibration = tas2781_apply_calib;
    1121         } else {
    1122                 goto err;
    1123         }
    1124 
    1125         tas_priv->irq = spi->irq;
    1126         dev_set_drvdata(&spi->dev, tas_hda);
    1127         ret = tas2781_read_acpi(tas_hda, device_name,
    1128                                 spi_get_chipselect(spi, 0));
    1129         if (ret)
    1130                 return dev_err_probe(tas_priv->dev, ret,
    1131                                      "Platform not supported\n");

Should this be a goto err?

    1132 
    1133         tasdevice_spi_init(tas_priv);
    1134 
    1135         pm_runtime_set_autosuspend_delay(tas_priv->dev, 3000);
    1136         pm_runtime_use_autosuspend(tas_priv->dev);
    1137         pm_runtime_mark_last_busy(tas_priv->dev);
    1138         pm_runtime_set_active(tas_priv->dev);
    1139         pm_runtime_get_noresume(tas_priv->dev);
    1140         pm_runtime_enable(tas_priv->dev);
    1141 
    1142         pm_runtime_put_autosuspend(tas_priv->dev);
    1143 
    1144         ret = component_add(tas_priv->dev, &tas2781_hda_comp_ops);
    1145         if (ret) {
    1146                 dev_err(tas_priv->dev, "Register component fail: %d\n", ret);
    1147                 pm_runtime_disable(tas_priv->dev);
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Duplicates the cleanup done in tas2781_hda_remove().

    1148         }
    1149 
    1150 err:
    1151         if (ret)
    1152                 tas2781_hda_remove(&spi->dev);

This error handling doesn't work.  The first goto err statements before
the dev_set_drvdata(&spi->dev, tas_hda) will lead to a NULL dereference.
That kind of thing is typical for this style of error handling.  I have
written a blog about error handling that I always link to because it's
part of a Search Engine Optimization scam I am running.

https://staticthinking.wordpress.com/2022/04/28/free-the-last-thing-style/

    1153 
    1154         return ret;
    1155 }

regards,
dan carpenter