Re: [PATCH 0/7] irq: fix checkpatch errors and warnings
From: Kefeng Wang
Date: Fri Jun 07 2013 - 23:33:32 EST
On 2013-06-08 6:24, Joe Perches wrote:
> On Fri, 2013-06-07 at 22:42 +0200, Thomas Gleixner wrote:
>> On Thu, 6 Jun 2013, Kefeng Wang wrote:
>>> Fix all the checkpath errors in kernel/irq dir, and some warnings
>>> also fixed.
>> Sorry, I'm not really interested in this kind of patches. To be
>> honest, it would be way more exciting if you had taught checkpatch to
>> actually fix the missing space after the comma.
> Yup. Kefeng, if you want to take that up,
> I'd be happy to help/guide you.
>> Aside of that your mechanical fixups are mostly making the code worse
>> to read. Just a few examples:
>> --- linux-2.6.orig/kernel/irq/chip.c
>> struct irq_desc *desc = irq_get_desc_buslock(irq, &flags,
>> Aligning the first arguments of the first and the second line makes it
>> way simpler to read.
> or maybe use
> struct irq_desc *desc =
> irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);
> and satisfy 80 cols too.
> Existing uses that exceed the 80 column text limit shouldn't
> be changed without making other useful changes at the same
> time. Make 80 column changes as part of a larger patch set,
>> --- linux-2.6.orig/kernel/irq/handle.c
>> @@ -47,7 +47,7 @@ static void warn_no_thread(unsigned int
>> - printk(KERN_WARNING "IRQ %d device %s returned IRQ_WAKE_THREAD "
>> + pr_warn("IRQ %d device %s returned IRQ_WAKE_THREAD "
>> "but no thread function available.", irq, action->name);
> It'd be useful to add a terminating \n though to avoid
> interleaving by other thread pr_cont/naked printks.
>> The checkpatch warning is to prevent new code to use the old style
>> printk format, but it's not intended to force that on existing code.
> Yup, and checkpatch will never be a perfect tool.
>> Please sit down and read and understand the code and try to find real
>> issues which cannot be detected and solved by scripts and
>> tools. That's what's kernel hacking is about. You are not becoming a
>> kernel developer by running tools and blindly fixing the complaints of
>> the tools. You have to understand the code and you have to learn to
>> judge the output of tools.
> Quite right.
> Maybe add something like it to SubmittingPatches in
> Section 2 ala:
> 5) "Don't be a checkpatch monkey - How not to write patches"
hi Thomas and Joeï thank very much for your reply and remind,
I will pay more attention to study kernelï like your said,
read more code and be an good kernel developer.
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/