lkml.org 
[lkml]   [2011]   [Jul]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2 v3] Staging: comedi: fix printk() issue in adv_pci1710.c
On 19/07/11 16:07, Ryan Mallon wrote:
> On 19/07/11 15:50, Ravishankar wrote:
>> From: Ravishankar<ravi.shankar@greenturtles.in>
>>
>> This is a patch to the adv_pci1710.c file that fixes up a printk()
>> warning found by the checkpatch.pl tool
>>
>> Signed-off-by: Ravishankar<ravishankarkm32@gmail.com>
>> ---
>> KERN_CONT issue is fixed
>>
>> drivers/staging/comedi/drivers/adv_pci1710.c | 20
>> ++++++++++----------
>> 1 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
>> b/drivers/staging/comedi/drivers/adv_pci1710.c
>> index fd71cc6..093b9e6 100644
>> --- a/drivers/staging/comedi/drivers/adv_pci1710.c
>> +++ b/drivers/staging/comedi/drivers/adv_pci1710.c
>> @@ -1396,14 +1396,14 @@ static int pci1710_attach(struct
>> comedi_device *dev,
>> int i;
>> int board_index;
>>
>> - printk("comedi%d: adv_pci1710: ", dev->minor);
>> + printk(KERN_INFO "comedi%d: adv_pci1710: ", dev->minor);
>>
>> opt_bus = it->options[0];
>> opt_slot = it->options[1];
>>
>> ret = alloc_private(dev, sizeof(struct pci1710_private));
>> if (ret< 0) {
>> - printk(" - Allocation failed!\n");
>> + printk(KERN_CONT "\n");
>> + printk(KERN_ERR "Comedi%d: adv_pci1710: Allocation failed\n",
>> + dev->minor);
>
> This still isn't correct. The initial printk (KERN_INFO above) has no
> trailing newline. Here you are almost doing it correctly, except that
> you are adding an extra newline. This error message will look like this:
>
> comedi0: adv_pci1710:
> comedi0: adv_pci1710: Allocation failed
>
> Which is just silly. The printk's below are broken in that you are
> using KERN_ERR where you should be using KERN_CONT.
>
> As it stands the code is a little tricky to follow since the trailing
> newline gets added at the end unless there is an error when it gets
> added in place. It might be better just to print the full message,
> including the "comedi%d: adv_pci1710:" bit, at each printk call site
> and get rid of the whole KERN_CONT nonsense altogether. Alternatively
> you could create a comedi_printk function (or use pr_fmt) which wraps
> this up.

In fact, looking at the code again, the printks should probably just be
replaced with the dev_printk functions which should print the necessary
device name information.

~Ryan



\
 
 \ /
  Last update: 2011-07-20 01:47    [W:0.052 / U:0.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site