Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Trond Myklebust <trondmy@hammerspace.com>
CC:     Anna Schumaker <anna.schumaker@netapp.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "bcodding@redhat.com" <bcodding@redhat.com>
Subject: Re: [PATCH] xprtrdma: Fix a maybe-uninitialized compiler warning
Thread-Topic: [PATCH] xprtrdma: Fix a maybe-uninitialized compiler warning
Thread-Index: AQHXzlh5ESpT5Pt9IEKLG0l4cvaW6avtNIkAgANANACAAAIwAA==
Date:   Tue, 2 Nov 2021 16:50:59 +0000
Message-ID: <72A2B417-F76C-4896-AD91-77226C25BF74@oracle.com>
References: <2b32d41cf6a502918a685447cd749c4b1cb0d16d.1635685588.git.bcodding@redhat.com>
 <3EC776BC-8E37-477B-98BB-C9DE163EFA52@oracle.com>
 <d29f109c50126168ed83108ed4297c56771070ce.camel@hammerspace.com>
In-Reply-To: <d29f109c50126168ed83108ed4297c56771070ce.camel@hammerspace.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?znMJdIf8WszGsBeFvUBl7xIlAGV6oDCz7zq+SACWwtAAE5MlUzuRQ3u3urzi?=
 =?us-ascii?Q?WJQ8txqqgScudBYbKHZKQJr5hUh3wREel3bOYHIXZ7e/+3olKEDJJwQAV9G6?=
 =?us-ascii?Q?ySm4D34BLA3vd21z7ZZF/ftFYa5+IfutbuXk3QVSDPZDB5Hkindp4oDh9EbQ?=
 =?us-ascii?Q?yMVvb29Ap3HIEscgRMyuVCwIeATfQJVxIYv5oN0dVxXOWoOiJr18u1XmBb58?=
 =?us-ascii?Q?yKx0shb71ti4tgR9AIwD3I3oOwm8qLVti+6PO7GPvol5V+yJ+EHpEGdH2Qsy?=
 =?us-ascii?Q?iRVkF1FbvqRrTuG1sNjncdvqsgefJyInKn64tSqNORTBffjFxvclP4+WnqmH?=
 =?us-ascii?Q?BXBig6AQcmBhDXnVNpbjIzvWVFxd5sR2FUPL0Bf96UHfwCOMl2XHrrrPLawE?=
 =?us-ascii?Q?lcnFDdpHFecmGVthHa+lNiS9gFIkPvmogx6Hk229EO69j++hrqRhlZZ8Iyep?=
 =?us-ascii?Q?s8173jPcbt7aU49ufLt2I89bZLwuFpdD4Si2pvXrQNkvhUjQWQ6fHITmMpMI?=
 =?us-ascii?Q?dGLb8nAoHGtkYJmJNbqc6LdAadrizS8FIkUDr2zoZmienWEgHOMQqSOZSFHF?=
 =?us-ascii?Q?t+uXgjMnGO5njTTXIH1hecz272WTE2YrlAjydOQSw/uyWSSPiRUnvrFf1Xhb?=
 =?us-ascii?Q?3ReNuIjKdByl9oRjfYJbmrdo6UukhefymG6bPXLFDlrxas5Ag4eN9Syz2pfK?=
 =?us-ascii?Q?6gEYlkKQk6gKHaS2h1Nc/ZELz93h87scl3RIQpMNZzDUm8rDBz2luhmGxCXZ?=
 =?us-ascii?Q?yTWaERnrc+XfH3S1x1sbu5b5DloWHMHWui8byYuV/82BIhHOdnQtLTi9VyAX?=
 =?us-ascii?Q?PtcAq4H/ir7CgtDM+LnIJMjLuLbg4WSl9oabCjFqL3jcKGtrTLZRhNwhQHbv?=
 =?us-ascii?Q?iv9XIKTz0+x266i1/+Unn9iBc9GXTo81qCI/GdjKPKE/P75xSP0xlrbJ8byg?=
 =?us-ascii?Q?sWpRX7DWIxyTqPs1Ap7H8O6nyebKgvYUmwTQ7Zw1C9DEhDO+hGYMdJyYXnmO?=
 =?us-ascii?Q?inBsSR1AJcXD2xY+unHOogXo2Ji94pcGP8heqgCiMefNcbhP2yekxZjkRLb0?=
 =?us-ascii?Q?VZO11JCx+Qd0u8OwHQPdQ8nDaxOB+hVWlf59h3DmVcBl/1wDwjyjwbCC9AWF?=
 =?us-ascii?Q?6BsF59aOukgyqLTEVEHTUpCn+N53nx7CxCxnnTjhdWineO5Rd01odDDXq5iV?=
 =?us-ascii?Q?OA9nmF07I7vmhTOVYCdwfoagXqkseQFTkrf7BAAJxbcoljG8M6PaGD11Z2st?=
 =?us-ascii?Q?Rf7Gj+92jOfrzp31AJIG2mMZWPLSw6HRxViPR0KKso27Mww0nv6bJ77b6eyt?=
 =?us-ascii?Q?rQfCg+7vGDfUo99KhhvM1Z5dp5VWDvVKxgn7OWFxAvs+X6FsvGvMaXKModdF?=
 =?us-ascii?Q?V9ZpJMxu8Lu8uKXNzEOGT073/9eR9ek+5RUiVF3J2yR6mFchoArTIX6Ugfl+?=
 =?us-ascii?Q?hlcF4bxyEjHus6vzzPW4ccCnrVgtl/E8586002dkdekmrBkCG0I7gYnaVZS8?=
 =?us-ascii?Q?kL/reN5bCMNR4A7KOoq5wPEqSe/2JVKCWA1gh5JYJ0vP6nvzDvR0WzFGsKht?=
 =?us-ascii?Q?r8oliiU0/tRTrUwWqF++XrjBinvMKjZB9mX1s4yRSvnkY2vShY2CggX6cR21?=
 =?us-ascii?Q?Ksnx5SEVGBHWa8LODdsOe5s=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <5CEF98AE9ACDDD44960C2E3E5880133B@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4688.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9d69faf3-7c99-4cbb-f3f6-08d99e20f27c
X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Nov 2021 16:50:59.3067
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 5zsUrsPh6rK2AloB3t9XMIqGMovzSQ/wHusm5G51tmoU6wnnTlPLffnpGbAguQWWw2TsDwE47UILp2bW3HKNYg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB2693
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10156 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 phishscore=0 malwarescore=0
 mlxscore=0 suspectscore=0 mlxlogscore=999 adultscore=0 spamscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2111020096
X-Proofpoint-GUID: NIieLkrGd7n8lS0-iSoxOhsYUJhYTUBO
X-Proofpoint-ORIG-GUID: NIieLkrGd7n8lS0-iSoxOhsYUJhYTUBO
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Nov 2, 2021, at 12:43 PM, Trond Myklebust <trondmy@hammerspace.com> wr=
ote:
>=20
> On Sun, 2021-10-31 at 15:04 +0000, Chuck Lever III wrote:
>>=20
>>=20
>>> On Oct 31, 2021, at 9:08 AM, Benjamin Coddington
>>> <bcodding@redhat.com> wrote:
>>>=20
>>> This minor fix-up keeps GCC from complaining that "last' may be used
>>> uninitialized", which breaks some build workflows that have been
>>> running
>>> with all warnings treated as errors.
>>>=20
>>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>>=20
>> Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
>>=20
>>=20
>>> ---
>>> net/sunrpc/xprtrdma/frwr_ops.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>=20
>>> diff --git a/net/sunrpc/xprtrdma/frwr_ops.c
>>> b/net/sunrpc/xprtrdma/frwr_ops.c
>>> index f700b34a5bfd..de813fa07daa 100644
>>> --- a/net/sunrpc/xprtrdma/frwr_ops.c
>>> +++ b/net/sunrpc/xprtrdma/frwr_ops.c
>>> @@ -503,7 +503,7 @@ static void frwr_wc_localinv_wake(struct ib_cq
>>> *cq, struct ib_wc *wc)
>>>  */
>>> void frwr_unmap_sync(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req
>>> *req)
>>> {
>>> -       struct ib_send_wr *first, **prev, *last;
>>> +       struct ib_send_wr *first, **prev, *last =3D NULL;
>>>         struct rpcrdma_ep *ep =3D r_xprt->rx_ep;
>>>         const struct ib_send_wr *bad_wr;
>>>         struct rpcrdma_mr *mr;
>=20
> Err... Definitely not sufficient.
>=20
> gcc is absolutely correct to complain here, because if req-
>> rl_registered is empty, then the whole rest of the function after that
> while() loop is invalid.

The callers ensure rl_registered is not empty.

This used to be preferred: don't add code to check conditions
that are known to be true. If that policy is different now,
then yes, this code will have to be restructured.


>>> @@ -608,7 +608,7 @@ static void frwr_wc_localinv_done(struct ib_cq
>>> *cq, struct ib_wc *wc)
>>>  */
>>> void frwr_unmap_async(struct rpcrdma_xprt *r_xprt, struct
>>> rpcrdma_req *req)
>>> {
>>> -       struct ib_send_wr *first, *last, **prev;
>>> +       struct ib_send_wr *first, *last =3D NULL, **prev;
>>>         struct rpcrdma_ep *ep =3D r_xprt->rx_ep;
>>>         struct rpcrdma_mr *mr;
>>>         int rc;
>>> --=20
>>> 2.31.1
>>>=20
>=20
> Ditto.
>=20
> --=20
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com

--
Chuck Lever