Re: [Fwd: [PATCH 1/2] string: Add stracpy and stracpy_pad mechanisms]

Joe Perches
2019-07-24T04:37:57Z

On Tue, 2019-07-23 at 23:27 -0500, Julia Lawall wrote:
>
> On Tue, 23 Jul 2019, Joe Perches wrote:
>
> > On Tue, 2019-07-23 at 22:54 -0500, Julia Lawall wrote:
> > > A seantic patch and the resulting output for the case where the third
> > > arugument is a constant is attached. Likewise the resulting output on a
> > > recent linux-next.
> > >
> > > julia
> >
> > Nice. Thanks Julia
> >
> > A couple issues:
> >
> > There is a problem with conversions with assignments
> > of strlcpy() so ideally the cocci script should make sure
> > any return value was not used before conversion.
> >
> > This is not a provably good conversion:
> >
> > drivers/s390/char/sclp_ftp.c
> > @@ -114,8 +114,7 @@ static int sclp_ftp_et7(const struct hmc
> > sccb->evbuf.mdd.ftp.length = ftp->len;
> > sccb->evbuf.mdd.ftp.bufaddr = virt_to_phys(ftp->buf);
> >
> > - len = strlcpy(sccb->evbuf.mdd.ftp.fident, ftp->fname,
> > - HMCDRV_FTP_FIDENT_MAX);
> > + len = stracpy(sccb->evbuf.mdd.ftp.fident, ftp->fname);
>
> Sorry, I don't understand the issue here. What specifically should I be
> looking for?

The return value of strlcpy differs from (strscpy or stracpy).

strlcpy returns the length of the src string
str[sa]cpy returns the length of the src string if it fits in dest
or -E2BIG if src is truncated by the size of dest
or -E2BIG if dest is 0 length

Any use of the strlcpy return value should not be converted
because the logic after an assignment or use of the return value
can not be assured to have the same behavior.

> > And:
> >
> > I would have expected the bit below to find and convert uses like
> > drivers/hwmon/adc128d818.c: strlcpy(info->type, "adc128d818", I2C_NAME_SIZE);
> > but it seems none of those were converted.
>
> OK, thanks. I will check on it.

Thanks again.

> julia
>
> > I don't know why.
> >
> > //------------------------------------------
> > @r1@
> > struct i1 *e1;
> > expression e2;
> > identifier f,i1,i2;
> > position p;
> > @@
> > \(strscpy\|strlcpy\)(e1->f, e2, i2)@p
> >
> > @@
> > identifier r1.i1,r1.i2;
> > type T;
> > @@
> > struct i1 { ... T i1[i2]; ... }
> >
> > @@
> > identifier f,i2;
> > expression e1,e2;
> > position r1.p;
> > @@
> > (
> > -strscpy
> > +stracpy
> > -strlcpy
> > +stracpy
> > )(e1->f, e2
> > - , i2
> > )@p
> > //------------------------------------------
> >
> > to find
> >
> >