linux-kernel - Re: [RFC 1/2] clockevents: return 'int' from clockevents_set

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]

Message-ID: <alpine.DEB.2.02.1405121140210.6261@ionos.tec.linutronix.de>
Date:	Mon, 12 May 2014 12:19:14 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Viresh Kumar <viresh.kumar@...aro.org>
cc:	linaro-kernel@...ts.linaro.org, linaro-networking@...aro.org,
	linux-kernel@...r.kernel.org, fweisbec@...il.com,
	arvind.chauhan@....com, daniel.lezcano@...aro.org,
	Nicolas Pitre <nicolas.pitre@...aro.org>
Subject: Re: [RFC 1/2] clockevents: return 'int' from
 clockevents_set_mode()

On Mon, 12 May 2014, Viresh Kumar wrote:
> clockevents_set_mode() calls dev->set_mode() which has return type of 'void'
> currently. Later patches would add another interface dev->set_dev_mode(), which
> will return int and clockevents_set_mode() must propagate failures returned from
> it.
> 
> This patch changes prototype of clockevents_set_mode() to return 'int' and fix
> caller sites as well. As most of the call sites maynot work if
> clockevents_set_mode() fails, do issue a WARN() on failures. All other are
> updated to return error codes.

Is this a speed coding contest? And did you even think about what I
suggested:

if (dev->set_dev_mode) {
   ret = dev->set_dev_mode(dev, mode);
   handle_return_value(ret);

Does that handle_return_value() mean that you sprinkle WARN_ONs all
over the place? Does it mean, that you change the return value
semantics of functions which happen to call clock_events_set_mode()
just because it now has a return value?

Hell no.

handle_return_value() means handle the return value right there. We
know what we want to set and we know what may fail and what not, so we
can explicitely WARN right there. And for those things which might
fail, we return the failure and handle it at the call site. 

I've warned you and I'm seriosly grumpy by now. You are just wasting
my time and I'm tired of that.

Find someone competent who reviews your patches, deals with you and
when they make sense sends them to me. Or just stay away from kernel/*
and find something else to wreckage.

Thanks,

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