From nobody Fri Jun 30 17:07:58 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Qt1xV6bTsz4kk5k for ; Fri, 30 Jun 2023 17:08:46 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Qt1xV5D34z3nJQ for ; Fri, 30 Jun 2023 17:08:46 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-wm1-f41.google.com with SMTP id 5b1f17b1804b1-3fa8ce2307dso25894565e9.2 for ; Fri, 30 Jun 2023 10:08:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688144925; x=1690736925; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=E2Q40cGcxgX8w4wa+2VH27FMMO1EuRCmYdfetdpyHxA=; b=O4vcNZW75kMNuueWtUdoYzwa2L0Dfi9cIZicy/DYC5zkm1WKOQAn08U3zv8JIp/rjV dhuMp6N80QmpjNxNMbOmdPWOz1u3FSfl65yVdlaUAz4yof0MLRankTf9tYxwUjTsbSP5 9/IkM2K+E9LEYKs50BKfAveGBinfrquNNe6ydYQshKIiTGM6MGllky6F+GpNRgIRGQz8 fXx4nd58aBEsOUVqECJ0Jki6zSa4GRi8JPvcX+jRVf0lwc4ywbXbWkJiRCW7D5bjGTfO HicbPvffJwmSks+5PeI9M+SRFIkKECfTO7Cdu1oylshzaLjB6N+le4SyYFQ+WaWDTaql uyeg== X-Gm-Message-State: AC+VfDxceLtVlQAYgWQ2T/4Gf2IYTaBi+S0FNXVjUDAqHlMGczsREFob GIh7i7f9r6ijp84OQqjd5vodvw== X-Google-Smtp-Source: ACHHUZ7xDOzzuPjbCeDj5M0PRCyaylVUguuEx/yfo/rThWcrdjg1iJu/k1bnKW/Zz+g2ReZ8ighNcg== X-Received: by 2002:a1c:7914:0:b0:3fa:984d:7e94 with SMTP id l20-20020a1c7914000000b003fa984d7e94mr3165257wme.18.1688144924731; Fri, 30 Jun 2023 10:08:44 -0700 (PDT) Received: from smtpclient.apple ([131.111.5.246]) by smtp.gmail.com with ESMTPSA id z20-20020a7bc7d4000000b003f90067880esm22392173wmk.47.2023.06.30.10.08.15 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 30 Jun 2023 10:08:28 -0700 (PDT) Content-Type: text/plain; charset=utf-8 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.600.7\)) Subject: Re: git: 56f3f2d2491e - main - libsecureboot: avoid set but not used errors From: Jessica Clarke In-Reply-To: <498f3ba2-dc7a-e7d3-626a-76ca68cee5b2@freebsd.org> Date: Fri, 30 Jun 2023 18:07:58 +0100 Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: References: <202306300652.35U6qpgP027126@gitrepo.freebsd.org> <667C347E-B7C7-405B-AFEC-F0A0FD0656F6@freebsd.org> <498f3ba2-dc7a-e7d3-626a-76ca68cee5b2@freebsd.org> To: Mitchell Horne X-Mailer: Apple Mail (2.3731.600.7) X-Rspamd-Queue-Id: 4Qt1xV5D34z3nJQ X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N On 30 Jun 2023, at 17:43, Mitchell Horne wrote: > On 6/30/23 11:42, Jessica Clarke wrote: >> On 30 Jun 2023, at 07:52, Simon J. Gerraty wrote: >>>=20 >>> The branch main has been updated by sjg: >>>=20 >>> URL: = https://cgit.FreeBSD.org/src/commit/?id=3D56f3f2d2491e30f369f9461c3cb2a366= bdffbe1d >>>=20 >>> commit 56f3f2d2491e30f369f9461c3cb2a366bdffbe1d >>> Author: Simon J. Gerraty >>> AuthorDate: 2023-06-30 06:52:17 +0000 >>> Commit: Simon J. Gerraty >>> CommitDate: 2023-06-30 06:52:17 +0000 >>>=20 >>> libsecureboot: avoid set but not used errors >>>=20 >>> Reviewed by: stevek >>> --- >>> lib/libsecureboot/openpgp/opgp_sig.c | 22 ++++++++++++---------- >>> lib/libsecureboot/vets.c | 7 +++++-- >>> 2 files changed, 17 insertions(+), 12 deletions(-) >>>=20 >>> diff --git a/lib/libsecureboot/openpgp/opgp_sig.c = b/lib/libsecureboot/openpgp/opgp_sig.c >>> index eec3469e3457..7f4e6fb98fd1 100644 >>> --- a/lib/libsecureboot/openpgp/opgp_sig.c >>> +++ b/lib/libsecureboot/openpgp/opgp_sig.c >>> @@ -464,20 +464,22 @@ verify_asc(const char *sigfile, int flags) >>> size_t n; >>> unsigned char *fdata, *sdata; >>> size_t fbytes, sbytes; >>> - >>> + >>> + fdata =3D NULL; >>> if ((sdata =3D read_file(sigfile, &sbytes))) { >>> n =3D strlcpy(pbuf, sigfile, sizeof(pbuf)); >>> - if ((cp =3D strrchr(pbuf, '.'))) >>> - *cp =3D '\0'; >>> - if ((fdata =3D read_file(pbuf, &fbytes))) { >>> - if (openpgp_verify(pbuf, fdata, fbytes, sdata, >>> - sbytes, flags)) { >>> - free(fdata); >>> - fdata =3D NULL; >>> + if (n < sizeof(pbuf)) { >>> + if ((cp =3D strrchr(pbuf, '.'))) >>> + *cp =3D '\0'; >>> + if ((fdata =3D read_file(pbuf, &fbytes))) { >>> + if (openpgp_verify(pbuf, fdata, fbytes, sdata, >>> + sbytes, flags)) { >>> + free(fdata); >>> + fdata =3D NULL; >>> + } >>> } >>> } >>> - } else >>> - fdata =3D NULL; >>> + } >>> free(sdata); >>> return (fdata); >>> } >>> diff --git a/lib/libsecureboot/vets.c b/lib/libsecureboot/vets.c >>> index 4375dfa76a89..12191097ff8c 100644 >>> --- a/lib/libsecureboot/vets.c >>> +++ b/lib/libsecureboot/vets.c >>> @@ -241,11 +241,14 @@ x509_cn_get(br_x509_certificate *xc, char = *buf, size_t len) >>> mc.vtable->start_cert(&mc.vtable, xc->data_len); >>> mc.vtable->append(&mc.vtable, xc->data, xc->data_len); >>> mc.vtable->end_cert(&mc.vtable); >>> - /* we don' actually care about cert status - just its name */ >>> + /* we don't actually care about cert status - just its name */ >>> err =3D mc.vtable->end_chain(&mc.vtable); >>>=20 >>> - if (!cn.status) >>> + if (!cn.status) { >>> buf =3D NULL; >>> + if (err =3D=3D 0) /* keep compiler happy */ >>> + buf =3D NULL; >> This is nonsense code. >> Jess >=20 > And yours is a needlessly abrasive and unhelpful reply. >=20 > Seriously Jess, if your choice is to read through and nit-pick = peoples' changes, then at least learn to deliver your feedback with an = ounce of tact. You=E2=80=99re right that I can be, and was, overly blunt, for which I = apologise, but I don=E2=80=99t think this kind of pointed attack on my = character on-list is an appropriate response. To reword and expand my = original email: A conditional assignment of the same value that has already been = assigned unconditionally is unidiomatic code generally regarded as a = code smell and should be avoided. In an ideal world that would give a = compiler warning, but it doesn=E2=80=99t for both GCC and Clang, perhaps = because the control dependency has ordering implications for atomics. = Since it appears you=E2=80=99re doing this just to insert a dummy read = of err, either err should be marked __unused or it should be removed = entirely and the return value of end_chain explicitly marked as unused = by casting it to void. Jess > Mitchell >=20 >>> + } >>> return (buf); >>> }