On Tue, Feb 20, 2018 at 08:26:45PM +0000, Winkler, Tomas wrote: > > > > On Mon, 2018-02-19 at 11:43 +0000, Winkler, Tomas wrote: > > > > All local variable declarations must be in the beginning of the > > > > function. > > > > > > Who says? > > > > It is coherent how we have everything else. > I will have to care about its value out of the scope where the variable existence is not relevant. > > > It is much easier to see the stack allocation this way when the allocation is > > only done in the beginning of each function. If you really need to do such > > pattern, then it would be a better idea to consider an additional helper > > function. > The code block decides whether to modify 'rc'. I'm not sure if additional function will make > the code cleaner, on the opposite. > > > > > > Your comment about not overriding error code is incorrect. > > > > > > Please explain? > > > > 'l_rc' overrides 'rc' in the case when both are non-zero. > > Yes, that's been the intention, we cannot return more than one value. > l_rc if set it has hire priority. > > > > > > > The value of 'rc' should be never overridden, which kind of supports > > > > to "just print" behavior that we had for a locality error. > > > > > > You are not consistent, you've agreed with propagating it to user > > > space. The error will be propagated in case of an error in locality > > > relinquish the device is pretty much in non functional state and > > > provious errors do not matter much, but rc value won't be modified if > > > locality_reliquish succeeds. > > > > Well, sometimes you fail to notice things and I failed to notice the collision > > above. The commit message does not describe why 'l_rc' > > overrides 'rc' in the case when both are non-zero. What was the reasoning, > > which made you end up with this priority order? Why is 'l_rc' more > > important than 'rc'? > > Because, it's fatal. I'm not sure it's matter much what the previous error was, it cannot be recovered > That's my understanding of this flow. > > > > My take is that does it really make sense have this change as part of a high > > priority bug fix that should be as localized as possible? > > Seems like a non-trivial problem by itself. > > Yes, the issue here is that also an error path can fail. Now what is the correct return value.. > > In any case, in order to resolve this dispute, I will post a version when the error is just prints out, > Once, however fatal the error is, it's very unlikely that it will happen. > Second the driver will find the device not responding in a subsequent command. > > Not perfect, but at least we will have functional driver. > > Thanks > Tomas > Please add my tested by to next version. Thanks. /Jarkko
