From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x535.google.com (mail-ed1-x535.google.com [IPv6:2a00:1450:4864:20::535]) by sourceware.org (Postfix) with ESMTPS id 9F0A83858C36 for ; Sun, 25 Dec 2022 01:53:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9F0A83858C36 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ed1-x535.google.com with SMTP id q15so10228862edb.1 for ; Sat, 24 Dec 2022 17:53:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=FT04f/qSmqJM7FNYn9b9uulj6C9nCcfEkeX1vtHgu3o=; b=QxraHppVocNJmnz8uZJlI+i66R4VlS2mBNsZThNPDbsjvlNj0w/KW8hSC81OVvNqqz Lww8YerYPRv87kdxw/6Hi7FGUqzAzIXIKCLf99V40DJ6xNU/3isgmpoCr3dYMMNUYla4 uXLiI8tx8G0LyM4BsIUjoqT/VX6qkk3mXaZx0Ja93fGm3sB4UD43jmcI2msSv7sA7uXK W4Z42mxNmY6j5MwQA/9fWJAfPZ3vCbSlW4QlpV18vXhSrZRqOpotd6u4gu745yI0UT4d 5qm82yIqdSjYWPgp6SknP7i7J9OxUg7BVa3VI6UrSVPs2jPf/nBj+OrmXO3hqUfPPNpG waKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=FT04f/qSmqJM7FNYn9b9uulj6C9nCcfEkeX1vtHgu3o=; b=iYDqrRHeVgspNn9mEuznCw2fgX5OjAUaXUEX17SZPB3EI7Gg3ghUMONZxDgjY1zgZ8 LZcMmalAV5P5KCoCqO+088RSLRQCYEdCaG6NFHhU7FE0jwRic4hKhm2+eHCTgs0cUWPI NAVdVYT6Cs43qYdfHKTX//LMw6HEW5u7crg6hDRjDjdMmE7ZbPcDj0IWZzFfJqxIjHEa 1V3mkuhotb7ibEDtsl6MVI4H0vSj4rIt8A0Ux+X36RJALc2hxOGZv5df8jjER/aqNPb7 EdKOu9hAZa8cexafeprTOC+nFJ/IW0FcCvh62Cfn7zt4pAgPddJz5zCoH5QpIGiZvmfj PT5Q== X-Gm-Message-State: AFqh2krcG4xlhmCEGWCaQDydSwC06Ragv6FqrA1K7wxVJ+8j/hrIO1/D ub8oom9M2Kj9Kby3XFw80eh1I3xRWfhuZM3HXcM= X-Google-Smtp-Source: AMrXdXsRNFsXk4xy0at7o4T3WE6LRLaAw0wCK/K2DC+fey6h/2rcgH8fvoyD0Nb3NIP4tYCf1VnzSSTsXJdufhtZBoc= X-Received: by 2002:a05:6402:2408:b0:47b:2514:f2e7 with SMTP id t8-20020a056402240800b0047b2514f2e7mr1715703eda.142.1671933191062; Sat, 24 Dec 2022 17:53:11 -0800 (PST) MIME-Version: 1.0 References: <91d159d1-d379-af67-6859-bf8e3fa14c72@gmail.com> In-Reply-To: <91d159d1-d379-af67-6859-bf8e3fa14c72@gmail.com> From: Noah Goldstein Date: Sat, 24 Dec 2022 17:52:59 -0800 Message-ID: Subject: Re: [PATCH 1/1] string: Add stpecpy(3) To: Alejandro Colomar Cc: Wilco Dijkstra , GNU C Library Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, Dec 23, 2022 at 4:26 PM Alejandro Colomar via Libc-alpha wrote: > > > > On 12/24/22 01:05, Alejandro Colomar wrote: > > > > > > On 12/24/22 00:24, Wilco Dijkstra wrote: > >> Hi Alex, > >> > >>> For that, we'd first need to discuss what is a typical scenario. > >> > >> Like copying/concatenating strings that fit within the buffer. > > >>> And also, it depends a lot on what the compiler can optimize. If I call > >>> strlcat(3) in a loop, I know that stpecpy(3) is going to be orders of magnitude > >>> faster. > >> > >> If you're trying to say that the 'strcat' variant is bad then yes absolutely - > > > > I must admit that it has good things, if you have a compiler that does the magic > > for you. GCC optimizes strcat(3) into stpcpy(3), so if you know that it will be > > optimized, it's not so bad, and the source code is cleaner to the eye. > > > >> it's better to inline in the compiler or avoid the 'strcat' versions altogether > >> (that's also why I would strongly suggest never to add more 'cat' variants). > > > > inlining in the compiler is a good solution. And yes, I agree on not adding cat > > variants, but at the same time, str functions are problematic in that they can't > > be chained, as opposed to stp functions. That problem shows itself in the > > snprintf(3) bugs I mentioned. Users need a way to catenate easily, even if not > > with a 'cat' function. > > > >> But that doesn't say anything about whether stpecpy is better than strlcpy. > >> > >>> If I call strlcpy(3) in a loop, doing what an ideal compiler might do, that > >>> might be something to benchmark, but we'd also need to discuss what is a good > >>> input for the benchmark. > >> > >> The typical case would be copying or concatenating smallish strings to a buffer. > > > > Okay, I'll try to prepare a benchmark. > > > >> > >>> In the OpenBSD definition of strlcpy(), I count 4 branches, and one of them is > >>> inside a while loop. So, I'd find it very surprising if strlcpy(3) outperformed > >>> stpecpy(3). > >> > >> If that really is the OpenBSD implementation then this proves my point that > >> non-standard string functions are often totally unoptimized. > > > > And not only that, but I find your version much more readable. I don't > > understand how the OpenBSD version was written that way, and hasn't been fixed > > so far. > > > >> > >> A basic implementation of strlcpy would use strlen and memcpy so it is fast > >> on every system without requiring any optimization: > >> > >> size_t > >> strlcpy (char *dst, const char *src, size_t size) > >> { > >> size_t len = strlen (src); > >> > >> if (size == 0) > >> return len; > >> size = len >= size ? size - 1 : len; > > > > I'd use a separate variable dlen, to differentiate it from size. Otherwise, it > > looks like an off-by-one bug just below, since writing at a [size] usually means > > writing past the array. > > > >> dst[size] = 0; > >> memcpy (dst, src, size); > >> return len; > >> } > > > > Then, to compare oranges to oranges, I'll provide the equivalently optimized > > stpecpy(3): > > > > char * > > stpecpy (char *dst, char *end, const char *restrict src) > > { > > size_t dsize; > > size_t dlen; > > size_t slen; > > > > slen = strlen(src); > > > > if (dst == end) > > return NULL; > > if (unlikely(dst == NULL)) > > return NULL; > > if (dst > end) > > unreachable(); > > dsize = end - dst; > > dlen = slen >= dsize ? dsize - 1 : slen; > > dst[dlen] = 0; > > return mempcpy(dst, src, dlen); > > } > > Sorry, I wrote a bug while optimizing: I forgot about the sentinel 'end' return. > Now I think it should be fine (anyway, I'll test it soon): > > char * > stpecpy (char *dst, char *end, const char *restrict src) > { > size_t dsize; > size_t dlen; > size_t slen = strlen (src); Imo move `dst == end` and `dst == NULL` check before strlen and change strlen to `strnlen(src, dsize)`. > bool trunc = false; > > if (dst == end) Out of curiosity what if `end < dst`? > return NULL; > if (dst == NULL) > return NULL; > dsize = end - dst; > trunc = (slen >= dsize); > dlen = trunc ? dsize - 1 : slen; > dst[dlen] = 0; > return mempcpy(dst, src, dlen) + trunc; > } > > This adds a '+' operation, so the difference compared to your strlcpy(3) is > smaller, but stpecpy() still wins, I think. > > > -- >