drivers: led: Fix uninitialized variable

[rruuaanng]

Fix uninitialized ret variables scanned by Coverity.

from:
#81958

[simonguinot]

Well, first there is actually no case where ret is tested/used uninitialized with the current code. This can only happen if the color_mapping is invalid. But it is tested by is31fl3194_check_config().

That being said, it is interesting to consolidate the code...

About this patch, it is not optimal to initialize ret with 0. This opens the door to update the LED even if the channels have not been updated.

Since there is no point in continuing at sending I2C I/Os if one of them fails, then I would prefer something like that:

                ret = i2c_reg_write_byte_dt(&config->bus, led_channels[i], value);
                if (ret != 0) 
                        goto err;
        }

        ret = i2c_reg_write_byte_dt(&config->bus,
                                    IS31FL3194_UPDATE_REG,
                                    IS31FL3194_UPDATE_VAL);

err:
        if (ret != 0) {
                LOG_ERR("%s: LED write failed: %d", dev->name, ret);
        }

        return ret;
}

[rruuaanng]

Sorry to have kept you waiting, I think your suggestion is very good. After ret is initialized to 0, we should remove the if, I will fix it in two commits :)

Edit

Hold on, we seem to have forgotten the i2c_reg_write_byte_dt above. Its main function is to execute i2c_reg_write_byte_dt in the if below after all the i2c_reg_write_byte_dt above succeed. It seems that it makes sense to initialize ret to 0.

[simonguinot]

Sorry I don't see what is wrong this the code example I proposed. The last call to i2c_reg_write_byte_dt is only executed if the color registers are updated.

[simonguinot]

Hi @rruuaanng. Thanks for taking care of this issue. Please see my comment above.

[jilaypandya]

Hi @rruuaanng, is it okay if we close this PR. Since #82679 is already merged, let's see if sca is happy with the solution or not :)

[rruuaanng]

Thank you for your reminder. I have closed it.

[rruuaanng]

I agree with @simonguinot suggestion, but it is likely to be regarded as an appearance change. I think we can change it in the future. That is, when there are other problems with the driver. So, thank you :--)