Re: [PATCH] [JFFS2] jffs2_start_garbage_collect_thread() returnvalue cleanup

[David Woodhouse]

On Tue, 2009-06-02 at 15:11 +0300, Gerard Lledo wrote:
> There is no user of this return value in the kernel.  Change it to return void
> instead.

NAK. I hate this type of patch.

A function _should_ return an error value indicating success or failure,
if there's _any_ chance that it (or a future rewrite of it) may fail.

It's up to the _callers_ to act on that result, or not, as they see fit.

If you don't do that, you end up with whole stacks of functions which
neither handle nor propagate errors -- and it's a _complete_ pain when
you later find that you _do_ have to make one of the innermost functions
return an error.

Gerard, if you persist in sending this kind of patch, please could I ask
you to send me a cloth doll of yourself? I would like to stick pins in
it next time I have to retrofit error handling to a stack of functions
that lack it.

It's not even as if this patch is going to give any real performance
improvement. It's a single register load in a once-per-mount code path.

There _may_ be some places where this kind of patch is reasonable -- but
the commit comment should have a clear explanation of why the return
value will _never_ be needed, rather than merely observing that it is
not _currently_ used. Or show that there is a _significant_ improvement
obtained by doing so, perhaps.

Andrew, may I suggest that you look for such justification in future
patches of this type?

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@xxxxxxxxx                              Intel Corporation


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