Messages in this thread | | | Date | Tue, 27 Apr 2010 09:54:46 +0200 | From | Michal Simek <> | Subject | Re: [PATCH 1/2] microblaze: add stack unwinder |
| |
Steven J. Magnani wrote: > On Wed, 2010-04-14 at 18:45 +0200, Michal Simek wrote: >>> diff -uprN a/arch/microblaze/kernel/traps.c >> b/arch/microblaze/kernel/traps.c >>> --- a/arch/microblaze/kernel/traps.c 2010-04-09 21:52:36.000000000 >>> +++ b/arch/microblaze/kernel/traps.c 2010-04-12 22:16:01.000000000 > [snip] >>> >>> - if (!stack) >>> - stack = (unsigned long *)&stack; >>> + if (fp == 0) { >>> + if (task) >>> + fp = ((struct thread_info *) >>> + (task->stack))->cpu_context.r1; >>> + else { >>> + /* Pick up caller of dump_stack() */ >>> + fp = (__u32)&sp - 8; >>> + } >>> + } >> just coding style. >> >> if (fp == 0) >> if (task) >> fp = ((struct thread_info *) >> (task->stack))->cpu_context.r1; >> else >> /* Pick up caller of dump_stack() */ >> fp = (__u32)&sp - 8; > > Do you feel strongly about this? I try to always use braces on if/else > clauses that have more than one line. I've found that the extra > characters are well worth the savings in debugging time when someone > tries to extend the clause and forgets to add the braces.
It will work. I like brackets too. I agree that can save a lot of time with debugging.
I just didn't like that inconsistency in if (task) part.
If is if (task) { ... } else {...} then I am ok with it if checkpatch.pl doesn't report it as warning.
Michal
-- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian
| |