Chain stpcpy(3) to terminate strings after mempcpy(3)

Message ID 20231204164706.2527766-1-alx@kernel.org
State New
Headers
Series Chain stpcpy(3) to terminate strings after mempcpy(3) |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed

Commit Message

Alejandro Colomar Dec. 4, 2023, 4:47 p.m. UTC
  This is simpler than casting to set to '\0', and uses less screen space.
The compiler should be able to optimize it out to a simple assignment,
so it shouldn't have performance problems.

Also, it has the benefit that the last byte is fortified with
_FORTIFY_SOURCE thanks to stpcpy(3), while doing ='\0' would be
unprotected.

In most cases strcpy(3) would be as good, since we're discarding the
return value, or copying an empty string.  However, consistently using
stpcpy(3) is, well, more consistent.  And it is simpler to think about
it, because we're chaining 'p' calls (mem*p*cpy(3) and st*p*cpy(3)).

Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 catgets/gencat.c                    |  3 +--
 elf/chroot_canon.c                  |  3 +--
 elf/dl-load.c                       |  8 ++++----
 intl/dcigettext.c                   |  2 +-
 io/ftw.c                            |  5 ++---
 libio/fileops.c                     |  2 +-
 locale/programs/locfile.c           |  4 ++--
 nis/nis_subr.c                      |  4 ++--
 nis/ypclnt.c                        |  8 ++++----
 posix/glob.c                        | 16 ++++++----------
 posix/wordexp.c                     |  6 +++---
 stdlib/canonicalize.c               |  3 +--
 string/argz-replace.c               |  2 +-
 string/xpg-strerror.c               |  2 +-
 sysdeps/mach/xpg-strerror.c         |  2 +-
 sysdeps/posix/libc_fatal.c          |  2 +-
 sysdeps/unix/sysv/linux/dl-origin.c |  2 +-
 sysdeps/unix/sysv/linux/ifaddrs.c   |  8 ++++----
 18 files changed, 37 insertions(+), 45 deletions(-)
  

Patch

diff --git a/catgets/gencat.c b/catgets/gencat.c
index 63bdbf86a6..481877f6ba 100644
--- a/catgets/gencat.c
+++ b/catgets/gencat.c
@@ -379,8 +379,7 @@  read_input_file (struct catalog *current, const char *fname)
 		    {
 		      int len = cnt - start;
 		      codeset = xmalloc (len + 1);
-		      *((char *) mempcpy (codeset, &this_line[start], len))
-			= '\0';
+		      stpcpy (mempcpy (codeset, &this_line[start], len), "");
 		    }
 		}
 	    }
diff --git a/elf/chroot_canon.c b/elf/chroot_canon.c
index 63a1ae6dbb..85c657e76a 100644
--- a/elf/chroot_canon.c
+++ b/elf/chroot_canon.c
@@ -111,8 +111,7 @@  chroot_canon (const char *chroot, const char *name)
 	      dest = rpath + dest_offset;
 	    }
 
-	  dest = mempcpy (dest, start, end - start);
-	  *dest = '\0';
+	  dest = stpcpy (mempcpy (dest, start, end - start), "");
 
 	  if (lstat (rpath, &st) < 0)
 	    {
diff --git a/elf/dl-load.c b/elf/dl-load.c
index 25ea4f7a4e..5406351022 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -537,7 +537,7 @@  fillin_rpath (char *rpath, struct r_search_path_elem **result, const char *sep,
 
 	  dirp->dirname = ((char *) dirp + sizeof (*dirp)
 			   + ncapstr * sizeof (enum r_dir_status));
-	  *((char *) __mempcpy ((char *) dirp->dirname, cp, len)) = '\0';
+	  stpcpy (__mempcpy ((char *) dirp->dirname, cp, len), "");
 	  dirp->dirnamelen = len;
 
 	  if (len > max_dirnamelen)
@@ -1539,11 +1539,11 @@  print_search_path (struct r_search_path_elem **list,
 #ifdef SHARED
 	    char *cp = __mempcpy (endp, capstr[cnt].str, capstr[cnt].len);
 	    if (cp == buf || (cp == buf + 1 && buf[0] == '/'))
-	      cp[0] = '\0';
+	      stpcpy (cp, "");
 	    else
-	      cp[-1] = '\0';
+	      stpcpy (cp - 1, "");
 #else
-	    *endp = '\0';
+	    stpcpy (endp, "");
 #endif
 
 	    _dl_debug_printf_c (first ? "%s" : ":%s", buf);
diff --git a/intl/dcigettext.c b/intl/dcigettext.c
index 27063886d2..c851a755e2 100644
--- a/intl/dcigettext.c
+++ b/intl/dcigettext.c
@@ -1110,7 +1110,7 @@  _nl_find_msg (struct loaded_l10nfile *domain_file,
 
 		      charset = (char *) alloca (len + 1);
 # if defined _LIBC || HAVE_MEMPCPY
-		      *((char *) mempcpy (charset, charsetstr, len)) = '\0';
+		      stpcpy (mempcpy (charset, charsetstr, len), "");
 # else
 		      memcpy (charset, charsetstr, len);
 		      charset[len] = '\0';
diff --git a/io/ftw.c b/io/ftw.c
index a72c7d5171..624c393f16 100644
--- a/io/ftw.c
+++ b/io/ftw.c
@@ -314,8 +314,7 @@  open_dir_stream (int *dfdp, struct ftw_data *data, struct dir_data *dirp)
 		  buf = newp;
 		}
 
-	      *((char *) __mempcpy (buf + actsize, d->d_name, this_len))
-		= '\0';
+	      stpcpy (__mempcpy (buf + actsize, d->d_name, this_len), "");
 	      actsize += this_len + 1;
 	    }
 
@@ -407,7 +406,7 @@  process_entry (struct ftw_data *data, struct dir_data *dir, const char *name,
       && !ftw_allocate (data, 2 * new_buflen))
     return -1;
 
-  *((char *) __mempcpy (data->dirbuf + data->ftw.base, name, namlen)) = '\0';
+  stpcpy (__mempcpy (data->dirbuf + data->ftw.base, name, namlen), "");
 
   int statres;
   if (dir->streamfd != -1)
diff --git a/libio/fileops.c b/libio/fileops.c
index 1c1113e339..63e41f89c8 100644
--- a/libio/fileops.c
+++ b/libio/fileops.c
@@ -302,7 +302,7 @@  _IO_new_file_fopen (FILE *fp, const char *filename, const char *mode,
 	      return NULL;
 	    }
 
-	  *((char *) __mempcpy (ccs, cs + 5, endp - (cs + 5))) = '\0';
+	  stpcpy (__mempcpy (ccs, cs + 5, endp - (cs + 5)), "");
 	  strip (ccs, ccs);
 
 	  if (__wcsmbs_named_conv (&fcts, ccs[2] == '\0'
diff --git a/locale/programs/locfile.c b/locale/programs/locfile.c
index 1fecca6520..88696a2f69 100644
--- a/locale/programs/locfile.c
+++ b/locale/programs/locfile.c
@@ -162,8 +162,8 @@  argument to `%s' must be a single character"),
 	    {
 	      char *newp = alloca (arg->val.str.lenmb + 1);
 
-	      *((char *) mempcpy (newp, arg->val.str.startmb,
-				  arg->val.str.lenmb)) = '\0';
+	      stpcpy (mempcpy (newp, arg->val.str.startmb,
+				  arg->val.str.lenmb), "");
 	      repertoire_name = newp;
 	    }
 	  break;
diff --git a/nis/nis_subr.c b/nis/nis_subr.c
index f52a6e20cb..2e17c8b8bd 100644
--- a/nis/nis_subr.c
+++ b/nis/nis_subr.c
@@ -45,7 +45,7 @@  nis_leaf_of_r (const_nis_name name, char *buffer, size_t buflen)
       return NULL;
     }
 
-  *((char *) __mempcpy (buffer, name, i)) = '\0';
+  stpcpy (__mempcpy (buffer, name, i), "");
 
   return buffer;
 }
@@ -81,7 +81,7 @@  nis_name_of_r (const_nis_name name, char *buffer, size_t buflen)
       return NULL;
     }
 
-  *((char *) __mempcpy (buffer, name, diff - 1)) = '\0';
+  stpcpy (__mempcpy (buffer, name, diff - 1), "");
 
   if (diff - 1 == 0)
     return NULL;
diff --git a/nis/ypclnt.c b/nis/ypclnt.c
index 19a53a1054..c5bac582d9 100644
--- a/nis/ypclnt.c
+++ b/nis/ypclnt.c
@@ -695,10 +695,10 @@  __xdr_ypresp_all (XDR *xdrs, struct ypresp_all_data *objp)
 	       if we don't modify the length. So add an extra NUL
 	       character to avoid trouble with broken code. */
 	    objp->status = YP_TRUE;
-	    *((char *) __mempcpy (key, resp.ypresp_all_u.val.key.keydat_val,
-				  keylen)) = '\0';
-	    *((char *) __mempcpy (val, resp.ypresp_all_u.val.val.valdat_val,
-				  vallen)) = '\0';
+	    stpcpy (__mempcpy (key, resp.ypresp_all_u.val.key.keydat_val,
+				  keylen), "");
+	    stpcpy (__mempcpy (val, resp.ypresp_all_u.val.val.valdat_val,
+				  vallen), "");
 	    xdr_free ((xdrproc_t) xdr_ypresp_all, (char *) &resp);
 	    if ((*objp->foreach) (objp->status, key, keylen,
 				  val, vallen, objp->data))
diff --git a/posix/glob.c b/posix/glob.c
index f2e359f385..69d8b4c119 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -546,7 +546,7 @@  __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 
           ++dirlen;
           drive_spec = __alloca (dirlen + 1);
-          *((char *) mempcpy (drive_spec, pattern, dirlen)) = '\0';
+          stpcpy (mempcpy (drive_spec, pattern, dirlen), "");
           /* For now, disallow wildcards in the drive spec, to
              prevent infinite recursion in glob.  */
           if (__glob_pattern_p (drive_spec, !(flags & GLOB_NOESCAPE)))
@@ -566,7 +566,7 @@  __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
             return GLOB_NOSPACE;
           malloc_dirname = 1;
         }
-      *((char *) mempcpy (newp, pattern, dirlen)) = '\0';
+      stpcpy (mempcpy (newp, pattern, dirlen), "");
       dirname = newp;
       ++filename;
 
@@ -808,8 +808,7 @@  __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
                   *p = '\0';
                 }
               else
-                *((char *) mempcpy (newp, dirname + 1, end_name - dirname - 1))
-                  = '\0';
+                stpcpy (mempcpy (newp, dirname + 1, end_name - dirname - 1), "");
               user_name = newp;
             }
 
@@ -869,7 +868,7 @@  __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
                 d = mempcpy (dirname, p->pw_dir, home_len);
                 if (end_name != NULL)
                   d = mempcpy (d, end_name, rest_len);
-                *d = '\0';
+                stpcpy (d, "");
 
                 free (prev_dirname);
 
@@ -932,13 +931,10 @@  __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 
       if (flags & GLOB_MARK && is_dir (dirname, flags, pglob))
         {
-          char *p;
           pglob->gl_pathv[newcount] = malloc (dirlen + 2);
           if (pglob->gl_pathv[newcount] == NULL)
             goto nospace;
-          p = mempcpy (pglob->gl_pathv[newcount], dirname, dirlen);
-          p[0] = '/';
-          p[1] = '\0';
+          stpcpy (mempcpy (pglob->gl_pathv[newcount], dirname, dirlen), "/");
           if (__glibc_unlikely (malloc_dirname))
             free (dirname);
         }
@@ -1455,7 +1451,7 @@  glob_in_dir (const char *pattern, const char *directory, int flags,
       names->name[cur] = malloc (len + 1);
       if (names->name[cur] == NULL)
         goto memory_error;
-      *((char *) mempcpy (names->name[cur++], pattern, len)) = '\0';
+      stpcpy (mempcpy (names->name[cur++], pattern, len), "");
     }
 
   result = GLOB_NOMATCH;
diff --git a/posix/wordexp.c b/posix/wordexp.c
index 994d79161f..e0563ffbc5 100644
--- a/posix/wordexp.c
+++ b/posix/wordexp.c
@@ -116,7 +116,7 @@  w_addmem (char *buffer, size_t *actlen, size_t *maxlen, const char *str,
 
   if (buffer != NULL)
     {
-      *((char *) __mempcpy (&buffer[*actlen], str, len)) = '\0';
+      stpcpy (__mempcpy (&buffer[*actlen], str, len), "");
       *actlen += len;
     }
 
@@ -1733,7 +1733,7 @@  envsubst:
 			    goto no_space;
 			  }
 
-			*(char *) __mempcpy (newval, value, p - value) = '\0';
+			stpcpy (__mempcpy (newval, value, p - value), "");
 			if (free_value)
 			  free (value);
 			value = newval;
@@ -1759,7 +1759,7 @@  envsubst:
 			    goto no_space;
 			  }
 
-			*(char *) __mempcpy (newval, value, p - value) = '\0';
+			stpcpy (__mempcpy (newval, value, p - value), "");
 			if (free_value)
 			  free (value);
 			value = newval;
diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c
index 7e421ef115..9e4d8cf55a 100644
--- a/stdlib/canonicalize.c
+++ b/stdlib/canonicalize.c
@@ -293,8 +293,7 @@  realpath_stk (const char *name, char *resolved, struct realpath_bufs *bufs)
               dest = rname + dest_offset;
             }
 
-          dest = __mempcpy (dest, start, startlen);
-          *dest = '\0';
+          dest = stpcpy (__mempcpy (dest, start, startlen), "");
 
           char *buf;
           ssize_t n;
diff --git a/string/argz-replace.c b/string/argz-replace.c
index 28df058a4b..fead4a32e4 100644
--- a/string/argz-replace.c
+++ b/string/argz-replace.c
@@ -31,7 +31,7 @@  str_append (char **to, size_t *to_len, const char *buf, const size_t buf_len)
 
   if (new_to)
     {
-      *((char *) __mempcpy (new_to + *to_len, buf, buf_len)) = '\0';
+      stpcpy (__mempcpy (new_to + *to_len, buf, buf_len), "");
       *to = new_to;
       *to_len = new_len;
     }
diff --git a/string/xpg-strerror.c b/string/xpg-strerror.c
index c3155d1934..7c81b5e69e 100644
--- a/string/xpg-strerror.c
+++ b/string/xpg-strerror.c
@@ -39,7 +39,7 @@  __xpg_strerror_r (int errnum, char *buf, size_t buflen)
 
       /* Terminate the string in any case.  */
       if (buflen > 0)
-	*((char *) __mempcpy (buf, estr, MIN (buflen - 1, estrlen))) = '\0';
+	stpcpy (__mempcpy (buf, estr, MIN (buflen - 1, estrlen)), "");
 
       return buflen <= estrlen ? ERANGE : 0;
     }
diff --git a/sysdeps/mach/xpg-strerror.c b/sysdeps/mach/xpg-strerror.c
index 36aa449ba5..e350c4efdd 100644
--- a/sysdeps/mach/xpg-strerror.c
+++ b/sysdeps/mach/xpg-strerror.c
@@ -70,7 +70,7 @@  __xpg_strerror_r (int errnum, char *buf, size_t buflen)
 
   /* Terminate the string in any case.  */
   if (buflen > 0)
-    *((char *) __mempcpy (buf, estr, MIN (buflen - 1, estrlen))) = '\0';
+    stpcpy (__mempcpy (buf, estr, MIN (buflen - 1, estrlen)), "");
 
   return buflen <= estrlen ? ERANGE : 0;
 }
diff --git a/sysdeps/posix/libc_fatal.c b/sysdeps/posix/libc_fatal.c
index f564d232bf..3ed56f9741 100644
--- a/sysdeps/posix/libc_fatal.c
+++ b/sysdeps/posix/libc_fatal.c
@@ -115,7 +115,7 @@  __libc_message_impl (const char *fmt, ...)
 	  char *wp = buf->msg;
 	  for (int cnt = 0; cnt < iovcnt; ++cnt)
 	    wp = mempcpy (wp, iov[cnt].iov_base, iov[cnt].iov_len);
-	  *wp = '\0';
+	  stpcpy (wp, "");
 
 	  __set_vma_name (buf, total, " glibc: fatal");
 
diff --git a/sysdeps/unix/sysv/linux/dl-origin.c b/sysdeps/unix/sysv/linux/dl-origin.c
index d87e89335d..30dd8643d8 100644
--- a/sysdeps/unix/sysv/linux/dl-origin.c
+++ b/sysdeps/unix/sysv/linux/dl-origin.c
@@ -47,7 +47,7 @@  _dl_get_origin (void)
       else if (len == 1)
 	memcpy (result, "/", 2);
       else
-	*((char *) __mempcpy (result, linkval, len - 1)) = '\0';
+	stpcpy (__mempcpy (result, linkval, len - 1), "");
     }
   else
     {
diff --git a/sysdeps/unix/sysv/linux/ifaddrs.c b/sysdeps/unix/sysv/linux/ifaddrs.c
index 0db9bb7847..b250e91b05 100644
--- a/sysdeps/unix/sysv/linux/ifaddrs.c
+++ b/sysdeps/unix/sysv/linux/ifaddrs.c
@@ -514,8 +514,8 @@  getifaddrs_internal (struct ifaddrs **ifap)
 		      if ((rta_payload + 1) <= sizeof (ifas[ifa_index].name))
 			{
 			  ifas[ifa_index].ifa.ifa_name = ifas[ifa_index].name;
-			  *(char *) __mempcpy (ifas[ifa_index].name, rta_data,
-					       rta_payload) = '\0';
+			  stpcpy (__mempcpy (ifas[ifa_index].name, rta_data,
+					       rta_payload), "");
 			}
 		      break;
 
@@ -715,8 +715,8 @@  getifaddrs_internal (struct ifaddrs **ifap)
 		      if (rta_payload + 1 <= sizeof (ifas[ifa_index].name))
 			{
 			  ifas[ifa_index].ifa.ifa_name = ifas[ifa_index].name;
-			  *(char *) __mempcpy (ifas[ifa_index].name, rta_data,
-					       rta_payload) = '\0';
+			  stpcpy (__mempcpy (ifas[ifa_index].name, rta_data,
+					       rta_payload), "");
 			}
 		      else
 			abort ();