Re: [PATCH 1/4] do_wait reorganization

[Linus Torvalds]

On Mon, 5 May 2008, Roland McGrath wrote:
>
> This breaks out the guts of do_wait into three subfunctions.
> The control flow is less nonobvious without so much goto.

I dunno.

I absolutely detest your propensity for multiple return values. You have 
"int ret" for the return value, and then you *also* have a "int *retval" 
for the return value.

Which is what, and why?

I really don't think this helps in the "nonobvious" department. 

Yes, I can see the comments, so I know what it's supposed to be about, but 
it still disturbs me.

I'm pretty sure it should be possible to return a positive value for 
"eligible but not available" and make do with just one return value, but 
if that is just not possible or too complicated, at least don't call it 
"retval" and have totally different semantics from the return value we 
return?

So for example, maybe it could just be count of eligible children, and we 
call could it "int *eligible", and then rather than initialize to -ECHILD, 
initialize to zero, and make the logic be

	if (!eligible)
		return -ECHILD;
	.. otherwise see if we can wait, return -EINTR or whatever it we 
	can't ..

which then looks like a fairly sane thing to do in all contexts (both the 
caller and the callee).

			Linus
--
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/