Re: [RFC 1/2] clockevents: return 'int' from clockevents_set_mode()

From: Thomas Gleixner
Date: Mon May 12 2014 - 06:19:08 EST


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