linux-kernel - Re: [PATCH] Staging: comedi: ssv_snp: fix checkpatch.pl warnings

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] [day] [month] [year] [list]

Date:	Fri, 10 Aug 2012 15:08:54 +0300
From:	Güngör Erseymen <gelurine@...il.com>
To:	H Hartley Sweeten <hartleys@...ionengravers.com>
Cc:	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"abbotti@....co.uk" <abbotti@....co.uk>,
	"fmhess@...rs.sourceforge.net" <fmhess@...rs.sourceforge.net>,
	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Staging: comedi: ssv_snp: fix checkpatch.pl warnings

On Thu, Aug 9, 2012 at 7:27 PM, H Hartley Sweeten
<hartleys@...ionengravers.com> wrote:
> You have a typo in the subject for this patch.
> "ssv_snp" should be "ssv_dnp"
Hello Hartley,

I missed that typo. This is my first patch for linux kernel, i am a bit nervous.

>> diff --git a/drivers/staging/comedi/drivers/ssv_dnp.c b/drivers/staging/comedi/drivers/ssv_dnp.c
>> index 84b9f2a..4cd0f1b 100644
>> --- a/drivers/staging/comedi/drivers/ssv_dnp.c
>> +++ b/drivers/staging/comedi/drivers/ssv_dnp.c
>> @@ -177,7 +177,7 @@ static int dnp_attach(struct comedi_device *dev, struct comedi_devconfig *it)
>>       struct comedi_subdevice *s;
>>       int ret;
>>
>> -     printk(KERN_INFO "comedi%d: dnp: ", dev->minor);
>> +     pr_info("comedi%d: dnp: ", dev->minor);
>
> Where possible, fixes like this should use the dev_printk versions.
> For instance, this one would be:
>
>         dev_info(dev->class_dev, "dnp:");
>
> But, there is a cleaner fix for this file. See below.
>
>>       dev->board_name = board->name;
>>
>> @@ -195,7 +195,7 @@ static int dnp_attach(struct comedi_device *dev, struct comedi_devconfig *it)
>>       s->insn_bits = dnp_dio_insn_bits;
>>       s->insn_config = dnp_dio_insn_config;
>>
>> -     printk("attached\n");
>> +     pr_info("attached\n");
>
> There are only two printk's in this file, both in the "attach" routine.
>
> The first one does not have a "\n" and the function could exit without
> ever terminating the message.
>
> A better fix would be to merge the two messages into one "attached" message
> at the end of the function. You could also use the dev->board_name instead of
> the open coded string. Something like:
>
>         dev_info(dev->class_dev, "%s: attached\n", dev->board_name);
>
> Also, the message should be moved so that it's the last thing that happens
> before the function returns.

Thanks for advice, i will send a new patch with changes.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/