From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x531.google.com (mail-ed1-x531.google.com [IPv6:2a00:1450:4864:20::531]) by sourceware.org (Postfix) with ESMTPS id 8ABD83858D33 for ; Sun, 25 Dec 2022 22:32:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8ABD83858D33 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-x531.google.com with SMTP id u18so12383519eda.9 for ; Sun, 25 Dec 2022 14:32: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=qS3i+Jp/ZML4NuG5+5X8F7Nw9gpvpPE2efquAh/QecM=; b=AY5Y8fy7kvV7+pKbwKjTr+7WQT6O0CnOocgmGC5Zq8bTrstWtreQFrx/uF5N+qx5EA MqnkQMBUfzvR4yWz+tu7CXgka0fypkAM/3lV2GgISG5vn1Pom3tBex4tM7Pch5V6DOMq 3MO7ZO7plJ80Dn/o88Qfui+vmAFBTsH/xe6/atpCL038IPwqMcrN/dq5t9nfmI23yojm 6pCwqXdym8jNQz4aH0Ebd58Rr1mrFNSg5hvei04d16rkzm2dNV2/42YR9TYCvny58Q+j 0bRBdNPKB4gR/b9C3DmZtCjoZQI9ksHrvdZ5M8rNUtXCzbodSi98hLwWcMxmHS+mgyBN OilA== 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=qS3i+Jp/ZML4NuG5+5X8F7Nw9gpvpPE2efquAh/QecM=; b=uOB6j4pmIZUuwxlOnb9hUuDQNM2AFmSDs+Gbo8B86CFG4hdb5xgklXEwtN37M5hPxr 0pgXWFn/eKr6gkqAi+CZIUsAAmoycCVEVD+Ne50TKRyD5mo2QPfNZKonMN16gIBRJsUs 9U1Dz68999fu4FRLlAgnSR2/JSyzJyItXpkN3oQBFKILQyu8VxjJNxzzLM9UbUPqMbvm QUUdY9CLGbKzEtoi98iTUmieJZbZ4aKX5f4R+soHmFkn4Uenp2CEC6JH6MvJcaXXF3o5 FNbSIQPnuQEXglIcPbcpdKHNIRsJhxdTukGAkr45OjWqaHYRz6JZJDy3m13KX6bBI0l8 RGqw== X-Gm-Message-State: AFqh2koNhCTT2HsUF40ONY3MsbCsRxc/eNR05F1aIpaYOOzo6E97966v Nux2cdPN7DeWODaryANYxLk2Cu/PFvCXXaWW++GAjhgMgkw= X-Google-Smtp-Source: AMrXdXsPiw1nnMa/N/JlbHKUvSz4G9z13COpxvN54ZPx1peEdbKmaQp1eWbGOqTouft5Ni1PAKAzueXIHv/etuEviwY= X-Received: by 2002:aa7:c758:0:b0:47d:fc80:e566 with SMTP id c24-20020aa7c758000000b0047dfc80e566mr1874531eds.388.1672007530590; Sun, 25 Dec 2022 14:32:10 -0800 (PST) MIME-Version: 1.0 References: <91d159d1-d379-af67-6859-bf8e3fa14c72@gmail.com> <703b1fca-27ae-d1c7-863b-0573296fdb43@gmail.com> In-Reply-To: <703b1fca-27ae-d1c7-863b-0573296fdb43@gmail.com> From: Noah Goldstein Date: Sun, 25 Dec 2022 14:31: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.6 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 Sun, Dec 25, 2022 at 6:37 AM Alejandro Colomar wrote: > > Hi Noah, > > On 12/25/22 02:52, Noah Goldstein wrote: > > >> 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 > > > That's a valid optimization. Having strlen(3) before has the advantage that you > make sure that your strings are strings, as strlcpy(3) does. But since we're > inventing stpecpy(3), we can choose to be faster. If anyone wants to instrument > their code, they can add a temporary wrapper that does that. > > > and change strlen to `strnlen(src, dsize)`. > > About strnlen(3), I have doubts. Isn't strlen(3) faster for the common case of > no truncation or little truncation? strnlen(3) would optimize for the case > where you truncate by a large difference. It's faster if strlen(s) <= strnlen(s, N) (maybe up to N + 32). But generally I think of it like `qsort`. Most data gets n * log(n) behavior but still it's worth preventing the worst case for minor constant cost. > > >> bool trunc = false; > >> > >> if (dst == end) > > > > Out of curiosity what if `end < dst`? > > The behavior is undefined. That's by design. In the definition of stpecpy(3) I > have currently in libstp, I even tell the compiler to optimize on that condition: > > You could probably optimize out one of the branches along the line of: if((dst - 1UL) >= (end - 1UL)) { // if dst == NULL, then dst - 1UL -> SIZE_MAX and must be >= any value. // if dst == end, then (dst - 1UL) >= (end - 1UL) will be true. return NULL; } > > alx@asus5775:~/src/alx/libstp$ grepc -tfd stpecpy > ./include/stp/stpe/stpecpy.h:21: > inline char *stp_nullable > stpecpy(char *stp_nullable dst, char *end, const char *restrict src) > { > bool trunc; > size_t dsize, dlen, slen; > > slen = strlen(src); > > if (dst == end) > return end; > if (stp_unlikely(dst == NULL)) // Allow chaining with stpeprintf(). > return NULL; > stp_impossible(dst > end); > > dsize = end - dst; > trunc = (slen >= dsize); > dlen = trunc ? dsize - 1 : slen; > dst[dlen] = '\0'; > > return mempcpy(dst, src, dlen) + trunc; > } > alx@asus5775:~/src/alx/libstp$ grepc -tm stp_impossible > ./include/stp/_compiler.h:14: > #define stp_impossible(e) do \ > { \ > if (e) \ > stp_unreachable(); \ > } while (0) > alx@asus5775:~/src/alx/libstp$ grep -rnC1 define.stp_unreachable > include/stp/_compiler.h-28-#if defined(unreachable) > include/stp/_compiler.h:29:# define stp_unreachable() unreachable() > include/stp/_compiler.h-30-#else > include/stp/_compiler.h:31:# define stp_unreachable() __builtin_unreachable() > include/stp/_compiler.h-32-#endif > > > I'd do that for glibc, but I don't see any facility. Maybe we should add an > __impossible() macro to document UB, and help the compiler. Does it result in any improved codegen? If not seems like making it fail more noisily is always a win. > > Cheers, > > Alex > > >> 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. > >> > >> > >> -- > >> > > -- >