Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 17 Sep 2020 14:09:55 +0200
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Dmitry Osipenko <digetx@gmail.com>
Cc:     Jonathan Hunter <jonathanh@nvidia.com>,
        Laxman Dewangan <ldewangan@nvidia.com>,
        Wolfram Sang <wsa@the-dreams.de>,
        =?utf-8?B?TWljaGHFgiBNaXJvc8WCYXc=?= <mirq-linux@rere.qmqm.pl>,
        Andy Shevchenko <andy.shevchenko@gmail.com>,
        linux-i2c@vger.kernel.org, linux-tegra@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 27/34] i2c: tegra: Check errors for both positive and
 negative values
Message-ID: <20200917120955.GF3515672@ulmo>
References: <20200908224006.25636-1-digetx@gmail.com>
 <20200908224006.25636-28-digetx@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="7cXxibKNEnJOqEg4"
Content-Disposition: inline
In-Reply-To: <20200908224006.25636-28-digetx@gmail.com>
User-Agent: Mutt/1.14.6 (2020-07-11)
Precedence: bulk


--7cXxibKNEnJOqEg4
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Sep 09, 2020 at 01:39:59AM +0300, Dmitry Osipenko wrote:
> The driver's code is inconsistent in regards to the error values checking.
> The correct way should be to check both positive and negative values.
> This patch cleans up the error-checks in the code. Note that the
> pm_runtime_get_sync() could return positive value on success, hence only
> relevant parts of the code are changed by this patch.
>=20
> Reviewed-by: Micha=C5=82 Miros=C5=82aw <mirq-linux@rere.qmqm.pl>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Why? All of these functions "return 0 on success or a negative error
code on failure", don't they?

I would actually argue that all of the other checks are wrong. As you
mention yourself, some functions may decide to return positive values on
success to convey some extra information, so making this just check for
non-zero carries a risk of its own.

Thierry

--7cXxibKNEnJOqEg4
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAl9jUhEACgkQ3SOs138+
s6FoyQ//el+v4DlNBwE4rVBy7+LlFoHacHj1SffuSYl1l9sMcMCXXgupnlarJ2IR
JGDCtKtduxxm6MY5SORv95PTtRjNjOk2q7IaKn7uftDIH+GMVtH6y/7AIlSI3zle
YctRopASVvzNrf//KLqo6L1PqvyoAqW/Wcy30fihMT56n3ira/GxhJaBzJxcT4ss
aVDiiqqqKF3YwKiiiTvFkrLLWVzqe75KuRPJTTsvWI0OedPG60pWwJmfVWFTnsPU
gPQalW22mSd8ephR9CKxUgthkt+qLIcxlgJJmYYfp823NDdBPh56w54wJpyzUP5P
buuz4ngg2Wyxi77Rdhb1FIJOzcobtuRYqyDhrMX7J6OqgupP7gds34F99JHXw1Zs
bp/e5JJU5+80BqRVtQOMK0/+Bygdzd+5RUraYDHHu1F1cAUd3bN6DrchmsHI2XMt
DT4zxsukBteiTk9cCAf85U0Hi41urs4XOfA4bhiowtowjvtyRi2iGmseQdlt322g
nXdTQDQFSmPEBpamhFjU7h7C/N+N0vg8dsQngJY3dW28JpF1KM3QMzghN/OzqVTw
IIlyPgdMueoHET8FwPnm7zqcAb59iiVla0m6FOpRTCwFpwUE96MHNw/LRBFqDwx/
iCV2/MgCEG2iNytK96PFaA4HFOSakQYq/XF/XhtfExURwA2S8Ss=
=MhMV
-----END PGP SIGNATURE-----

--7cXxibKNEnJOqEg4--