Re: [PATCH v2 06/15] ACPI: LPSS: fix some coding style issues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andy,

On 2021/3/27 16:17, Andy Shevchenko wrote:


On Saturday, March 27, 2021, Xiaofei Tan <tanxiaofei@xxxxxxxxxx
<mailto:tanxiaofei@xxxxxxxxxx>> wrote:

    Fix some coding style issues reported by checkpatch.pl
    <http://checkpatch.pl>, including
    following types:

    WARNING: simple_strtol is obsolete, use kstrtol instead
    WARNING: Missing a blank line after declarations



First of all, two changes ==> two patches.


I thought it would be better to include more simple cleanup in one patch.



    Signed-off-by: Xiaofei Tan <tanxiaofei@xxxxxxxxxx
    <mailto:tanxiaofei@xxxxxxxxxx>>
    ---
     drivers/acpi/acpi_lpss.c | 4 +++-
     1 file changed, 3 insertions(+), 1 deletion(-)

    diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
    index be73974..2df231e 100644
    --- a/drivers/acpi/acpi_lpss.c
    +++ b/drivers/acpi/acpi_lpss.c
    @@ -187,7 +187,7 @@ static void byt_i2c_setup(struct
    lpss_private_data *pdata)

            /* Expected to always be true, but better safe then sorry */
            if (uid_str)
    -               uid = simple_strtol(uid_str, NULL, 10);
    +               uid = kstrtol(uid_str, NULL, 10);


How did you test this? Is there any guarantee that input is
null-terminated number?

Where is the check of returned value from `kstrtol()`?

Yes, you haven’t tested that at all. Don’t submit patches you are not
able to test and haven’t tested.


It's my fault. Sorry for that, i will fix it.

NAK.


            /* Detect I2C bus shared with PUNIT and ignore its d3 status */
            status = acpi_evaluate_integer(handle, "_SEM", NULL,
    &shared_host);
    @@ -377,6 +377,7 @@ static const struct acpi_device_id
    acpi_lpss_device_ids[] = {
     static int is_memory(struct acpi_resource *res, void *not_used)
     {
            struct resource r;
    +
            return !acpi_dev_resource_memory(res, &r);
     }

    @@ -1200,6 +1201,7 @@ static int acpi_lpss_poweroff_noirq(struct
    device *dev)
            if (pdata->dev_desc->resume_from_noirq) {
                    /* This is analogous to the
    acpi_lpss_suspend_noirq() case. */
                    int ret = acpi_lpss_do_poweroff_late(dev);
    +
                    if (ret)
                            return ret;
            }
    --
    2.8.1



--
With Best Regards,
Andy Shevchenko






[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux