Re: coding style (was Re: [PATCH][2.5] UTF-8 support in console)

Larry McVoy (lm@bitmover.com)
Sat, 31 May 2003 08:39:40 -0700


On Sat, May 31, 2003 at 04:01:50PM +0100, Dave Jones wrote:
> Saving a line over readability is utterly bogus.

I agree 100%. If you have anything more complex than

if (error) return (error);

I want it to look like

if ((expr) || (expr2) || (expr3)) {
return (error);
}

> Just look at some of the crap we have in devfs..

No kidding, look at the nested if, that's insane.

> if (fs_info->devfsd_task == NULL) return (TRUE);
> if (devfsd_queue_empty (fs_info) && fs_info->devfsd_sleeping) return TRUE;
> if ( is_devfsd_or_child (fs_info) ) return (FALSE);
> set_current_state (TASK_UNINTERRUPTIBLE);
> add_wait_queue (&fs_info->revalidate_wait_queue, &wait);
> if (!devfsd_queue_empty (fs_info) || !fs_info->devfsd_sleeping)
> if (fs_info->devfsd_task) schedule ();
> remove_wait_queue (&fs_info->revalidate_wait_queue, &wait);
> __set_current_state (TASK_RUNNING);
> return (TRUE);

I took a pass at this, I think this is better (note the use of 1/2 tabs
as "continuation" lines, that's a Sun thing and it works pretty well:

if ((fs_info->devfsd_task == NULL) ||
(devfsd_queue_empty(fs_info) && fs_info->devfsd_sleeping)) {
return (TRUE);
}
if (is_devfsd_or_child(fs_info)) return (FALSE);
set_current_state (TASK_UNINTERRUPTIBLE);
add_wait_queue (&fs_info->revalidate_wait_queue, &wait);
if ((!devfsd_queue_empty (fs_info) || !fs_info->devfsd_sleeping) &&
fs_info->devfsd_task) {
schedule();
}
remove_wait_queue(&fs_info->revalidate_wait_queue, &wait);
__set_current_state(TASK_RUNNING);
return (TRUE);

-- 
---
Larry McVoy              lm at bitmover.com          http://www.bitmover.com/lm
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/