From patchwork Mon Dec 4 16:47:14 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alejandro Colomar X-Patchwork-Id: 81292 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 58C5D384DEE3 for ; Mon, 4 Dec 2023 16:47:47 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by sourceware.org (Postfix) with ESMTPS id 567E2385782C for ; Mon, 4 Dec 2023 16:47:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 567E2385782C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kernel.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 567E2385782C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=139.178.84.217 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701708455; cv=none; b=pkKnVPnP1ApXzb5pB3n13IV9Xen4qX3vz5k5wkKzdkKkQ+z55G/sue5jMBXdD0lP1W/YU1yArXN6peNr+Wonl37OhkMvRhT0uppRlXRXyNHJnwhzq5XjNL5ORhAjGXghFDEzDopzSss9vwT4e8aLNb5/03jOjwvJK5mGdbUaimM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701708455; c=relaxed/simple; bh=2dh2wa4QQngkjw/tnV8Z+opfJzTQuTO/uzYDfSOg6RE=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=cCKrzWxERRX9MCejZffTQsxdG75f7tkOssCfQD1nvk/1pG3P54BaK9nlXBPTAfnq2BxnGm0yvsKdZVJ9z7D+LswzYRCFwctyX3iz0Q9GRR7YrjHIMlyuIP5ntCW8wCxTrlyXMj3Wn2dhDjG6xSHT1gTCkiGnAdq49hJv98dcxrQ= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id B3C0A60FA6 for ; Mon, 4 Dec 2023 16:47:32 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AB262C433CA; Mon, 4 Dec 2023 16:47:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1701708452; bh=2dh2wa4QQngkjw/tnV8Z+opfJzTQuTO/uzYDfSOg6RE=; h=Date:From:To:Cc:Subject:From; b=k1dxQe2IIDf8QDNlObn34T/jbR7CKoWDwJmyFptS/f2Qe2i5+lg7eUYrqWVjZS/3k S0wkDAYaaIRR6jC64C/W3Cn0oFYYL5Ad2/GAxyhkXeo+kl6i4NSZKj0/5tAFTMkiOv Zjh4tbGrbe//5+TmfDkyLtMo8mITLJEl6af14vUoSjwv/lxXF5qIV+WeBFnRl0zl6a 2zRnrl8iNg8SX7l3J8WvWHYsDAlpOyuRQEMJ6tapnnoLVerQ8DMEmYHiL2871Oo1Uw as86EKneW+pBJdBBLrBV34cBge+AaCQ1vbpy8H5fQziObB3jHkz9AFpf0rNItWdtsj 58kzqyvaREnnw== Date: Mon, 4 Dec 2023 17:47:14 +0100 From: Alejandro Colomar To: libc-alpha@sourceware.org Cc: Alejandro Colomar Subject: [PATCH] Chain stpcpy(3) to terminate strings after mempcpy(3) Message-ID: <20231204164706.2527766-1-alx@kernel.org> MIME-Version: 1.0 Content-Disposition: inline X-Mailer: git-send-email 2.42.0 X-Spam-Status: No, score=-9.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org 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 --- 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(-) 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 ();