On Tue, Nov 30, 2021 at 12:26:47PM +0100, Martin Wilck wrote:
> On Mon, 2021-11-29 at 19:18 -0600, Benjamin Marzinski wrote:
> > On Fri, Nov 19, 2021 at 12:13:24AM +0100, mwilck@xxxxxxxx wrote:
> > > - out[0] = '1';
> > > - len = 1;
> > > - while ((p = memchr(vpd, ' ', vpd_len))) {
> > > + if ((err = fill_strbuf(&buf, '1', 1)) < 0)
> > > + return err;
> > > + while (vpd && (p = memchr(vpd, ' ', vpd_len))) {
> >
> > vpd should never be zero here unless it wraps around, which seems
> > very
> > unlikely. Is this just to make coverity happy, or do you mean
> > (*vpd)?
> >
>
> Coverity likes arguments for memchr() to be NULL-checked beforehand,
> but that wasn't the main reason.
>
> See below, there's "vpd = p". And p can (and will sooner or later) be
> NULL, because memchr() returns NULL if it doesn't find anything.
>
Right, but don't you only execute this while loop if p isn't NULL? Not
that it really matters. I'm fine with the code that way it is.
-Ben
>
> > -Ben
> >
> > > p_len = p - vpd;
> > > - if (len + p_len > out_len - 1) {
> > > - condlog(1, "%s: WWID overflow, type
> > > 1, %zu/%zu bytes required",
> > > - __func__, len + p_len,
> > > out_len);
> > > - p_len = out_len - len - 1;
> > > - }
> > > - memcpy(out + len, vpd, p_len);
> > > - len += p_len;
> > > - if (len >= out_len - 1) {
> > > - out[len] = '\0';
> > > - break;
> > > - }
> > > - out[len] = '_';
> > > - len ++;
> > > - if (len >= out_len - 1) {
> > > - out[len] = '\0';
> > > - break;
> > > - }
> > > + if ((err = __append_strbuf_str(&buf, (const
> > > char *)vpd,
> > > + p_len)) < 0)
> > > + return err;
> > > vpd = p;
>
> Here.
>
> Regards,
> Martin
>
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
