linux-kernel - Re: [PATCH] [JFFS2] jffs2_start_garbage_collect

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 24 Jun 2009 11:09:01 +0100
From:	David Woodhouse <dwmw2@...radead.org>
To:	Gerard Lledo <gerard.lledo@...il.com>
Cc:	akpm@...ux-foundation.org, linux-mtd@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [JFFS2] jffs2_start_garbage_collect_thread() return
 value cleanup

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@...el.com                              Intel Corporation

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