linux-kernel - Re: [PATCH] Staging : rts5208 : checkpatch.pl fixes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+DH7V_TW-yQkUTTAQZo=uegkCFbAZDzVNT8Z2uv1WAZZ7hRgQ@mail.gmail.com>
Date:   Mon, 26 Jun 2017 11:12:54 +0200
From:   Simo Koskinen <koskisoft@...il.com>
To:     Joe Perches <joe@...ches.com>
Cc:     gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Staging : rts5208 : checkpatch.pl fixes

On Fri, Jun 23, 2017 at 5:59 PM, Joe Perches <joe@...ches.com> wrote:
> On Fri, 2017-06-23 at 15:55 +0200, Simo Koskinen wrote:
>> Fixed some issues reported by checkpatch.pl script.
> []
>> diff --git a/drivers/staging/rts5208/rtsx.c b/drivers/staging/rts5208/rtsx.c
>> index b8177f5..ceef5fc 100644
>> --- a/drivers/staging/rts5208/rtsx.c
>> +++ b/drivers/staging/rts5208/rtsx.c
>> @@ -1009,7 +1009,7 @@ static void rtsx_remove(struct pci_dev *pci)
>>  {
>>       struct rtsx_dev *dev = pci_get_drvdata(pci);
>>
>> -     dev_info(&pci->dev, "rtsx_remove() called\n");
>> +     dev_info(&pci->dev, "%s called\n", "rtsx_remove()");
>
> This would be better as dev_dbg
True, I can change that...


>>
>>       quiesce_and_remove_host(dev);
>>       release_everything(dev);
>> diff --git a/drivers/staging/rts5208/rtsx_chip.c b/drivers/staging/rts5208/rtsx_chip.c
>> index 7f4107b..892b97a 100644
>> --- a/drivers/staging/rts5208/rtsx_chip.c
>> +++ b/drivers/staging/rts5208/rtsx_chip.c
>> @@ -616,8 +616,10 @@ int rtsx_reset_chip(struct rtsx_chip *chip)
>>               else
>>                       retval = rtsx_pre_handle_sdio_new(chip);
>>
>> -             dev_dbg(rtsx_dev(chip), "chip->need_reset = 0x%x (rtsx_reset_chip)\n",
>> -                     (unsigned int)(chip->need_reset));
>> +             dev_dbg(rtsx_dev(chip), "%s = 0x%x (%s)\n",
>> +                             "chip->need_reset",
>> +                             (unsigned int)(chip->need_reset),
>> +                             "rtsx_reset_chip");
>
> This and other changes that take part of the format
> and convert them to '"%s", substrings' are not good.
> checkpatch doesn't emit a warning for long formats.
The reason for changes were the following warning when run the
checkpatch.pl script:

WARNING: Prefer using '"%s...", __func__' to using 'rtsx_reset_chip',
this function's name, in a string
#619: FILE: rtsx_chip.c:619:
+        dev_dbg(rtsx_dev(chip), "chip->need_reset = 0x%x (rtsx_reset_chip)\n",

So it's not a good idea to fix these warnings?


On Fri, Jun 23, 2017 at 5:59 PM, Joe Perches <joe@...ches.com> wrote:
> On Fri, 2017-06-23 at 15:55 +0200, Simo Koskinen wrote:
>> Fixed some issues reported by checkpatch.pl script.
> []
>> diff --git a/drivers/staging/rts5208/rtsx.c b/drivers/staging/rts5208/rtsx.c
>> index b8177f5..ceef5fc 100644
>> --- a/drivers/staging/rts5208/rtsx.c
>> +++ b/drivers/staging/rts5208/rtsx.c
>> @@ -1009,7 +1009,7 @@ static void rtsx_remove(struct pci_dev *pci)
>>  {
>>       struct rtsx_dev *dev = pci_get_drvdata(pci);
>>
>> -     dev_info(&pci->dev, "rtsx_remove() called\n");
>> +     dev_info(&pci->dev, "%s called\n", "rtsx_remove()");
>
> This would be better as dev_dbg
>>
>>       quiesce_and_remove_host(dev);
>>       release_everything(dev);
>> diff --git a/drivers/staging/rts5208/rtsx_chip.c b/drivers/staging/rts5208/rtsx_chip.c
>> index 7f4107b..892b97a 100644
>> --- a/drivers/staging/rts5208/rtsx_chip.c
>> +++ b/drivers/staging/rts5208/rtsx_chip.c
>> @@ -616,8 +616,10 @@ int rtsx_reset_chip(struct rtsx_chip *chip)
>>               else
>>                       retval = rtsx_pre_handle_sdio_new(chip);
>>
>> -             dev_dbg(rtsx_dev(chip), "chip->need_reset = 0x%x (rtsx_reset_chip)\n",
>> -                     (unsigned int)(chip->need_reset));
>> +             dev_dbg(rtsx_dev(chip), "%s = 0x%x (%s)\n",
>> +                             "chip->need_reset",
>> +                             (unsigned int)(chip->need_reset),
>> +                             "rtsx_reset_chip");
>
> This and other changes that take part of the format
> and convert them to '"%s", substrings' are not good.
> checkpatch doesn't emit a warning for long formats.
>
>> diff --git a/drivers/staging/rts5208/sd.c b/drivers/staging/rts5208/sd.c
> []
>> @@ -910,8 +910,12 @@ static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir)
>>       int retval;
>>       bool ddr_rx = false;
>>
>> -     dev_dbg(rtsx_dev(chip), "sd_change_phase (sample_point = %d, tune_dir = %d)\n",
>> -             sample_point, tune_dir);
>> +     dev_dbg(rtsx_dev(chip), "%s (%s = %d, %s = %d)\n",
>> +                     "sd_change_phase",
>> +                     "sample_point",
>> +                     sample_point,
>> +                     "tune_dir",
>> +                     tune_dir);
>
> etc.
>