Hi Paul, On 3/11/23 20:29, Paul Eggert wrote: > From d40e2f92f3e50d13d87393bd30b2b4b20b89a2d6 Mon Sep 17 00:00:00 2001 > From: Paul Eggert <egg...@cs.ucla.edu> > Date: Sat, 11 Mar 2023 00:01:02 -0800 > Subject: [PATCH 1/6] Fix undefined behavior in change_field > > * lib/fields.c (change_field): Do not ever compute &newf[-1], > as behavior is undefined. Since we know that the string fits, > use memcpy rather than strlcpy.
I'd separate the UB fix, from the transformation to memcpy(3),
in two separate commits, since they are conceptually unrelated.
>
> Signed-off-by: Paul Eggert <egg...@cs.ucla.edu>
> ---
> lib/fields.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/lib/fields.c b/lib/fields.c
> index 0b5f91b2..3b119502 100644
> --- a/lib/fields.c
> +++ b/lib/fields.c
> @@ -90,17 +90,17 @@ void change_field (char *buf, size_t maxsize, const char
> *prompt)
> * makes it possible to change the field to empty, by
> * entering a space. --marekm
> */
> + char *bp = newf;
>
> - while (--cp >= newf && isspace (*cp));
> - cp++;
> + while (newf < cp && isspace (cp[-1])) {
> + cp--;
> + }
> *cp = '\0';
>
> - cp = newf;
> - while (('\0' != *cp) && isspace (*cp)) {
> - cp++;
> + while (isspace (*bp)) {
> + bp++;
> }
>
> - strlcpy (buf, cp, maxsize);
> + memcpy (buf, bp, cp + 1 - bp);
Regarding this transformation, I'd prefer transforming to strcpy(3).
It avoids the manual `cp + 1 - bp` calculation.
Thanks for the review and patches!
Cheers,
Alex
> }
> }
> -
> --
> 2.37.2
>
--
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
OpenPGP_signature
Description: OpenPGP digital signature
