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