Re: git: 56f3f2d2491e - main - libsecureboot: avoid set but not used errors
Date: Fri, 30 Jun 2023 14:41:34 UTC
On 6/29/23 11:52 PM, Simon J. Gerraty wrote: > The branch main has been updated by sjg: > > URL: https://cgit.FreeBSD.org/src/commit/?id=56f3f2d2491e30f369f9461c3cb2a366bdffbe1d > > commit 56f3f2d2491e30f369f9461c3cb2a366bdffbe1d > Author: Simon J. Gerraty <sjg@FreeBSD.org> > AuthorDate: 2023-06-30 06:52:17 +0000 > Commit: Simon J. Gerraty <sjg@FreeBSD.org> > CommitDate: 2023-06-30 06:52:17 +0000 > > libsecureboot: avoid set but not used errors > > Reviewed by: stevek > --- > lib/libsecureboot/openpgp/opgp_sig.c | 22 ++++++++++++---------- > lib/libsecureboot/vets.c | 7 +++++-- > 2 files changed, 17 insertions(+), 12 deletions(-) > > 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 = NULL; > if ((sdata = read_file(sigfile, &sbytes))) { > n = strlcpy(pbuf, sigfile, sizeof(pbuf)); > - if ((cp = strrchr(pbuf, '.'))) > - *cp = '\0'; > - if ((fdata = read_file(pbuf, &fbytes))) { > - if (openpgp_verify(pbuf, fdata, fbytes, sdata, > - sbytes, flags)) { > - free(fdata); > - fdata = NULL; > + if (n < sizeof(pbuf)) { > + if ((cp = strrchr(pbuf, '.'))) > + *cp = '\0'; > + if ((fdata = read_file(pbuf, &fbytes))) { > + if (openpgp_verify(pbuf, fdata, fbytes, sdata, > + sbytes, flags)) { > + free(fdata); > + fdata = NULL; > + } > } > } > - } else > - fdata = NULL; > + } > free(sdata); > return (fdata); Most of this change seems to be avoiding reading the "real" file if the filename from the signature file was too long to fit into pbuf which I think is a different change? > } > 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 = mc.vtable->end_chain(&mc.vtable); For cases like this I've removed the variable and used a (void) cast instead to indicate that the return value is intentionally unused. > - if (!cn.status) > + if (!cn.status) { > buf = NULL; > + if (err == 0) /* keep compiler happy */ > + buf = NULL; > + } > return (buf); > } > -- John Baldwin