On Mon, 2018-02-19 at 13:27 +0200, Jarkko Sakkinen wrote:
> On Wed, Feb 14, 2018 at 03:43:18PM +0200, Tomas Winkler wrote:
> > if (need_locality && chip->ops->relinquish_locality) {
> > - chip->ops->relinquish_locality(chip, chip-
> > >locality);
> > + /* this coud be on error path, don't override
> > error code */
> > + int l_rc = chip->ops->relinquish_locality(chip,
> > chip->locality);
>
> All local variable declarations must be in the beginning of the
> function.
>
> > +
> > + if (l_rc) {
> > + dev_err(&chip->dev, "%s:
> > relinquish_locality: error %d\n",
> > + __func__, l_rc);
> > + rc = l_rc;
> > + }
>
> Your comment about not overriding error code is incorrect.
>
> The value of 'rc' should be never overridden, which kind of supports
> to "just print" behavior that we had for a locality error.
>
> Is your fix somehow dependent on changing relinquish_locality()
> behavior? If not, please remove this change. If you want to
> contribute
> such behavioral change, you should make a separate patch of it.
>
> Now it's like a trojan horse bundled inside a bug fix.
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
[And while doing this noticed a flaw in my test suite:
https://github.com/jsakkine-intel/tpm2-scripts/issues/3]
/Jarkko
