From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x841.google.com (mail-qt1-x841.google.com [IPv6:2607:f8b0:4864:20::841]) by sourceware.org (Postfix) with ESMTPS id CBCF83844079 for ; Fri, 3 Jul 2020 19:53:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CBCF83844079 Received: by mail-qt1-x841.google.com with SMTP id i3so24630636qtq.13 for ; Fri, 03 Jul 2020 12:53:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:autocrypt :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=arhpAVOMc7TY/Jq7AJ4ZUbC4UYOGkoVpgfxpKIwsMH4=; b=Aa7wensY5Lg6TsGdhymDrjat49jxYv1GyrHzlHiifTmSEuBC4/z4+v3eKzchY17iV6 4WZOCc3OAlCSmkPdunHZQNwXoOPGEYlrilmOkpVaCwNNkqKfWoxYnuNxcuNpbTeRFitY rEek+3K+Gvx7ksAxXSn+bu+1GusORlB7BX4LCc8CjRX/RCK4iu0gajp2IpiGaAS2VUFp JCm4UR1QPXXgaHbGbvwCDCjzX7+/OxszGTuCdcNzQWbjnBw9HdwyLWf6+BtBGD53WShg xy1cK352CYZ5CJvtoYZbuQnmAZ40L+S8f9i5Ev6unMBmzw/erHH0SoV5t6ZxaLi0nWsp GhMQ== X-Gm-Message-State: AOAM532d3iZHIfoHbPhnVZ18GnZLRQWAhJvQlQf9Yv9YElDKQAd05qnH nCWvEuGWTmqV1vWuJw8yf3JVIg== X-Google-Smtp-Source: ABdhPJyBX5mLrN6QrCDiQKAlnFKM0/OZenUkHFxq7e3Dm32mY0xTNbNJHPLkQQCruG/8cKnftudk7Q== X-Received: by 2002:ac8:4b63:: with SMTP id g3mr37723945qts.229.1593806019287; Fri, 03 Jul 2020 12:53:39 -0700 (PDT) Received: from [192.168.1.4] ([177.194.48.209]) by smtp.googlemail.com with ESMTPSA id f203sm12049387qke.135.2020.07.03.12.53.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 03 Jul 2020 12:53:38 -0700 (PDT) Subject: Re: [PATCH v5 06/13] string: Implement strerror in terms of strerror_l To: Carlos O'Donell , libc-alpha@sourceware.org References: <20200619134352.297146-1-adhemerval.zanella@linaro.org> <20200619134352.297146-6-adhemerval.zanella@linaro.org> Cc: "Michael Kerrisk (man-pages)" From: Adhemerval Zanella Autocrypt: addr=adhemerval.zanella@linaro.org; prefer-encrypt=mutual; keydata= mQINBFcVGkoBEADiQU2x/cBBmAVf5C2d1xgz6zCnlCefbqaflUBw4hB/bEME40QsrVzWZ5Nq 8kxkEczZzAOKkkvv4pRVLlLn/zDtFXhlcvQRJ3yFMGqzBjofucOrmdYkOGo0uCaoJKPT186L NWp53SACXguFJpnw4ODI64ziInzXQs/rUJqrFoVIlrPDmNv/LUv1OVPKz20ETjgfpg8MNwG6 iMizMefCl+RbtXbIEZ3TE/IaDT/jcOirjv96lBKrc/pAL0h/O71Kwbbp43fimW80GhjiaN2y WGByepnkAVP7FyNarhdDpJhoDmUk9yfwNuIuESaCQtfd3vgKKuo6grcKZ8bHy7IXX1XJj2X/ BgRVhVgMHAnDPFIkXtP+SiarkUaLjGzCz7XkUn4XAGDskBNfbizFqYUQCaL2FdbW3DeZqNIa nSzKAZK7Dm9+0VVSRZXP89w71Y7JUV56xL/PlOE+YKKFdEw+gQjQi0e+DZILAtFjJLoCrkEX w4LluMhYX/X8XP6/C3xW0yOZhvHYyn72sV4yJ1uyc/qz3OY32CRy+bwPzAMAkhdwcORA3JPb kPTlimhQqVgvca8m+MQ/JFZ6D+K7QPyvEv7bQ7M+IzFmTkOCwCJ3xqOD6GjX3aphk8Sr0dq3 4Awlf5xFDAG8dn8Uuutb7naGBd/fEv6t8dfkNyzj6yvc4jpVxwARAQABtElBZGhlbWVydmFs IFphbmVsbGEgTmV0dG8gKExpbmFybyBWUE4gS2V5KSA8YWRoZW1lcnZhbC56YW5lbGxhQGxp bmFyby5vcmc+iQI3BBMBCAAhBQJXFRpKAhsDBQsJCAcDBRUKCQgLBRYCAwEAAh4BAheAAAoJ EKqx7BSnlIjv0e8P/1YOYoNkvJ+AJcNUaM5a2SA9oAKjSJ/M/EN4Id5Ow41ZJS4lUA0apSXW NjQg3VeVc2RiHab2LIB4MxdJhaWTuzfLkYnBeoy4u6njYcaoSwf3g9dSsvsl3mhtuzm6aXFH /Qsauav77enJh99tI4T+58rp0EuLhDsQbnBic/ukYNv7sQV8dy9KxA54yLnYUFqH6pfH8Lly sTVAMyi5Fg5O5/hVV+Z0Kpr+ZocC1YFJkTsNLAW5EIYSP9ftniqaVsim7MNmodv/zqK0IyDB GLLH1kjhvb5+6ySGlWbMTomt/or/uvMgulz0bRS+LUyOmlfXDdT+t38VPKBBVwFMarNuREU2 69M3a3jdTfScboDd2ck1u7l+QbaGoHZQ8ZNUrzgObltjohiIsazqkgYDQzXIMrD9H19E+8fw kCNUlXxjEgH/Kg8DlpoYJXSJCX0fjMWfXywL6ZXc2xyG/hbl5hvsLNmqDpLpc1CfKcA0BkK+ k8R57fr91mTCppSwwKJYO9T+8J+o4ho/CJnK/jBy1pWKMYJPvvrpdBCWq3MfzVpXYdahRKHI ypk8m4QlRlbOXWJ3TDd/SKNfSSrWgwRSg7XCjSlR7PNzNFXTULLB34sZhjrN6Q8NQZsZnMNs TX8nlGOVrKolnQPjKCLwCyu8PhllU8OwbSMKskcD1PSkG6h3r0AquQINBFcVGkoBEACgAdbR Ck+fsfOVwT8zowMiL3l9a2DP3Eeak23ifdZG+8Avb/SImpv0UMSbRfnw/N81IWwlbjkjbGTu oT37iZHLRwYUFmA8fZX0wNDNKQUUTjN6XalJmvhdz9l71H3WnE0wneEM5ahu5V1L1utUWTyh VUwzX1lwJeV3vyrNgI1kYOaeuNVvq7npNR6t6XxEpqPsNc6O77I12XELic2+36YibyqlTJIQ V1SZEbIy26AbC2zH9WqaKyGyQnr/IPbTJ2Lv0dM3RaXoVf+CeK7gB2B+w1hZummD21c1Laua +VIMPCUQ+EM8W9EtX+0iJXxI+wsztLT6vltQcm+5Q7tY+HFUucizJkAOAz98YFucwKefbkTp eKvCfCwiM1bGatZEFFKIlvJ2QNMQNiUrqJBlW9nZp/k7pbG3oStOjvawD9ZbP9e0fnlWJIsj 6c7pX354Yi7kxIk/6gREidHLLqEb/otuwt1aoMPg97iUgDV5mlNef77lWE8vxmlY0FBWIXuZ yv0XYxf1WF6dRizwFFbxvUZzIJp3spAao7jLsQj1DbD2s5+S1BW09A0mI/1DjB6EhNN+4bDB SJCOv/ReK3tFJXuj/HbyDrOdoMt8aIFbe7YFLEExHpSk+HgN05Lg5TyTro8oW7TSMTk+8a5M kzaH4UGXTTBDP/g5cfL3RFPl79ubXwARAQABiQIfBBgBCAAJBQJXFRpKAhsMAAoJEKqx7BSn lIjvI/8P/jg0jl4Tbvg3B5kT6PxJOXHYu9OoyaHLcay6Cd+ZrOd1VQQCbOcgLFbf4Yr+rE9l mYsY67AUgq2QKmVVbn9pjvGsEaz8UmfDnz5epUhDxC6yRRvY4hreMXZhPZ1pbMa6A0a/WOSt AgFj5V6Z4dXGTM/lNManr0HjXxbUYv2WfbNt3/07Db9T+GZkpUotC6iknsTA4rJi6u2ls0W9 1UIvW4o01vb4nZRCj4rni0g6eWoQCGoVDk/xFfy7ZliR5B+3Z3EWRJcQskip/QAHjbLa3pml xAZ484fVxgeESOoaeC9TiBIp0NfH8akWOI0HpBCiBD5xaCTvR7ujUWMvhsX2n881r/hNlR9g fcE6q00qHSPAEgGr1bnFv74/1vbKtjeXLCcRKk3Ulw0bY1OoDxWQr86T2fZGJ/HIZuVVBf3+ gaYJF92GXFynHnea14nFFuFgOni0Mi1zDxYH/8yGGBXvo14KWd8JOW0NJPaCDFJkdS5hu0VY 7vJwKcyHJGxsCLU+Et0mryX8qZwqibJIzu7kUJQdQDljbRPDFd/xmGUFCQiQAncSilYOcxNU EMVCXPAQTteqkvA+gNqSaK1NM9tY0eQ4iJpo+aoX8HAcn4sZzt2pfUB9vQMTBJ2d4+m/qO6+ cFTAceXmIoFsN8+gFN3i8Is3u12u8xGudcBPvpoy4OoG Message-ID: <7be335d3-2cf2-c920-a245-3ed3c36366e5@linaro.org> Date: Fri, 3 Jul 2020 16:53:36 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-14.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Jul 2020 19:53:41 -0000 On 02/07/2020 16:44, Carlos O'Donell wrote: > On 6/19/20 9:43 AM, Adhemerval Zanella wrote: >> Checked on x86-64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu, >> and s390x-linux-gnu. > > OK for master if you accept the NEWS changes suggested. > > Please make sure to follow up with the linux man pages project to update > their strerror documentation, particularly now that strerror is now > MT-Safe, which is a good step forward. Ack. > > Tested-by: Carlos O'Donell > Reviewed-by: Carlos O'Donell > >> --- >> NEWS | 4 ++++ >> include/string.h | 4 ++++ >> string/strerror.c | 22 ++-------------------- >> string/strerror_l.c | 15 ++++++++++----- >> sysdeps/mach/strerror_l.c | 4 +++- >> 5 files changed, 23 insertions(+), 26 deletions(-) >> >> diff --git a/NEWS b/NEWS >> index df03a34657..ec39b6e056 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -63,6 +63,10 @@ Deprecated and removed features, and other changes affecting compatibility: >> compatibility symbols to support old binaries. All programs should use >> strerror or strerror_r instead. >> >> +* Both strerror and strerror_l now share the same internal buffer, meaning >> + that the returned string pointer might be invalidated or contents might >> + be overwritten in subsequent calls of either symbol. > > Suggest: > > * Both strerror and strerror_l now share the same internal buffer in the calling > thread, meaning that the returned string pointer may be invalided or contents > might be overwritten on subsequent calls in the same thread or if the thread > is terminated. Ack, I just adjusted the line wrap to fit on 78 columns and add the note it makes the strerror MT-safe: * Both strerror and strerror_l now share the same internal buffer in the calling thread, meaning that the returned string pointer may be invalided or contents might be overwritten on subsequent calls in the same thread or if the thread is terminated. It makes strerror MT-safe. > > Notes: > - If the thread is terminated then __libc_thread_freeres will free the storage > via __glibc_tls_internal_free. > - It is only within the calling thread that this matters. It makes strerror > MT-safe. I have add this implementation detail notes on commit message. > >> + >> Changes to build and runtime requirements: >> >> * powerpc64le requires GCC 7.4 or newer. This is required for supporting >> diff --git a/include/string.h b/include/string.h >> index 4d622f1c03..3a33327cc0 100644 >> --- a/include/string.h >> +++ b/include/string.h >> @@ -4,6 +4,7 @@ >> /* Some of these are defined as macros in the real string.h, so we must >> prototype them before including it. */ >> #include >> +#include > > OK. > >> >> extern void *__memccpy (void *__dest, const void *__src, >> int __c, size_t __n); >> @@ -50,6 +51,8 @@ extern int __ffs (int __i) __attribute__ ((const)); >> >> extern char *__strerror_r (int __errnum, char *__buf, size_t __buflen); >> >> +extern char *__strerror_l (int __errnum, locale_t __loc); >> + > > OK > >> /* Called as part of the thread shutdown sequence. */ >> void __strerror_thread_freeres (void) attribute_hidden; >> >> @@ -113,6 +116,7 @@ libc_hidden_proto (memmem) >> extern __typeof (memmem) __memmem; >> libc_hidden_proto (__memmem) >> libc_hidden_proto (__ffs) >> +libc_hidden_proto (__strerror_l) > > OK. > >> >> #if IS_IN (libc) >> /* Avoid hidden reference to IFUNC symbol __explicit_bzero_chk. */ >> diff --git a/string/strerror.c b/string/strerror.c >> index 283ab70f91..35c749016e 100644 >> --- a/string/strerror.c >> +++ b/string/strerror.c >> @@ -15,29 +15,11 @@ >> License along with the GNU C Library; if not, see >> . */ >> >> -#include >> -#include >> #include >> -#include >> - >> -/* Return a string describing the errno code in ERRNUM. >> - The storage is good only until the next call to strerror. >> - Writing to the storage causes undefined behavior. */ >> -libc_freeres_ptr (static char *buf); >> +#include >> >> char * >> strerror (int errnum) >> { >> - char *ret = __strerror_r (errnum, NULL, 0); >> - int saved_errno; >> - >> - if (__glibc_likely (ret != NULL)) >> - return ret; >> - saved_errno = errno; >> - if (buf == NULL) >> - buf = malloc (1024); >> - __set_errno (saved_errno); >> - if (buf == NULL) >> - return _("Unknown error"); >> - return __strerror_r (errnum, buf, 1024); >> + return __strerror_l (errnum, __libc_tsd_get (locale_t, LOCALE)); > > OK. > >> } >> diff --git a/string/strerror_l.c b/string/strerror_l.c >> index 309f42e66b..017bd14b99 100644 >> --- a/string/strerror_l.c >> +++ b/string/strerror_l.c >> @@ -20,8 +20,7 @@ >> #include >> #include >> #include >> -#include >> -#include >> +#include >> >> static __thread char *last_value; >> >> @@ -38,8 +37,9 @@ translate (const char *str, locale_t loc) >> >> /* Return a string describing the errno code in ERRNUM. */ >> char * >> -strerror_l (int errnum, locale_t loc) >> +__strerror_l (int errnum, locale_t loc) >> { >> + int saved_errno = errno; >> char *err = (char *) __get_errlist (errnum); >> if (__glibc_unlikely (err == NULL)) >> { >> @@ -48,11 +48,16 @@ strerror_l (int errnum, locale_t loc) >> translate ("Unknown error ", loc), errnum) == -1) >> last_value = NULL; >> >> - return last_value; >> + err = last_value; >> } >> + else >> + err = (char *) translate (err, loc); >> >> - return (char *) translate (err, loc); >> + __set_errno (saved_errno); >> + return err; >> } >> +weak_alias (__strerror_l, strerror_l) >> +libc_hidden_def (__strerror_l) > > OK. > >> >> void >> __strerror_thread_freeres (void) >> diff --git a/sysdeps/mach/strerror_l.c b/sysdeps/mach/strerror_l.c >> index f514af341e..b6c9fdbe80 100644 >> --- a/sysdeps/mach/strerror_l.c >> +++ b/sysdeps/mach/strerror_l.c >> @@ -42,7 +42,7 @@ translate (const char *str, locale_t loc) >> >> /* Return a string describing the errno code in ERRNUM. */ >> char * >> -strerror_l (int errnum, locale_t loc) >> +__strerror_l (int errnum, locale_t loc) >> { >> int system; >> int sub; >> @@ -86,6 +86,8 @@ strerror_l (int errnum, locale_t loc) >> >> return (char *) translate (es->subsystem[sub].codes[code], loc); >> } >> +weak_alias (__strerror_l, strerror_l) >> +libc_hidden_def (__strerror_l) > > OK. > >> >> /* This is called when a thread is exiting to free the last_value string. */ >> void >> > >