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

[Larry McVoy (lm@bitmover.com)]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/