From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Date: Wed, 18 Jan 2017 17:43:20 +0000 Subject: Re: alpha: Checking source code positions for the setting of error codes Message-Id: <20170118174320.GE1555@ZenIV.linux.org.uk> List-Id: References: <986573fa-e636-f864-14dc-5e65a31b454e@users.sourceforge.net> <20170118141103.GD1555@ZenIV.linux.org.uk> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: SF Markus Elfring Cc: linux-alpha@vger.kernel.org, Ivan Kokshaysky , Jan-Benedict Glaw , Matt Turner , Nicolas Pitre , Richard Cochran , Richard Henderson , Thomas Gleixner , LKML , kernel-janitors@vger.kernel.org On Wed, Jan 18, 2017 at 04:41:10PM +0100, SF Markus Elfring wrote: > >> A local variable was set to an error code in two cases before a concrete > >> error situation was detected. Thus move the corresponding assignment into > >> an if branch to indicate a software failure there. > >> > >> This issue was detected by using the Coccinelle software. > > > > Why the hell is that an issue? > > * Can misplaced variable assignments result in unwanted run time consequences > because of the previous approach for a control flow specification? More like the opposite. load constant to register test branch usually not taken is considerably cheaper than test branch usually taken Something like if (unlikely(foo)) { err = -ESOMETHING; goto sod_off; } would be more or less on par (and quite possibly would be compiled into the same code - depends upon the scheduling details for processor, but speculative load of constant can be an optimization). However, that has an effect of splattering the source with tons of those unlikely() *and* visually cluttering the common path. > * How do you think about to achieve that error codes will only be set > after a specific software failure was detected? Sounds like an arbitrary requirement, TBH... Again, loading a constant into register tends to be cheap and easy to combine with other instructions at CPU pipeline level. If anything, this pattern is a microoptimization, often in spots that are not on hotpaths by any stretch of imagination. But estimating whether a given place is on a hot path takes a lot more delicate analysis than feasible for cocci scripts. And visual cluttering of the common execution path remains - it doesn't matter for compiler, but it can matter a lot for human readers.