From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 3170 invoked from network); 21 Jan 2021 16:32:13 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 21 Jan 2021 16:32:13 -0000 Received: (qmail 14049 invoked by uid 550); 21 Jan 2021 16:32:10 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 14025 invoked from network); 21 Jan 2021 16:32:09 -0000 Date: Thu, 21 Jan 2021 17:31:52 +0100 From: Natanael Copa To: Rich Felker Cc: musl@lists.openwall.com, "Alex Xu (Hello71)" Message-ID: <20210121173152.6819b23e@ncopa-desktop.lan> In-Reply-To: <20210121161807.GN23432@brightrain.aerifal.cx> References: <20210121140240.83405-1-alex_y_xu.ref@yahoo.ca> <20210121140240.83405-1-alex_y_xu@yahoo.ca> <20210121165000.61205767@ncopa-desktop.lan> <20210121161807.GN23432@brightrain.aerifal.cx> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-alpine-linux-musl) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [musl] [PATCH] don't set errno in free On Thu, 21 Jan 2021 11:18:08 -0500 Rich Felker wrote: > On Thu, Jan 21, 2021 at 04:50:00PM +0100, Natanael Copa wrote: > > On Thu, 21 Jan 2021 09:02:40 -0500 > > "Alex Xu (Hello71)" wrote: > > > > > busybox echo fails if free sets errno, which madvise does on old > > > kernels. > > > --- > > > src/malloc/mallocng/free.c | 14 ++++++++++++-- > > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > > > diff --git a/src/malloc/mallocng/free.c b/src/malloc/mallocng/free.c > > > index 40745f97..82836815 100644 > > > --- a/src/malloc/mallocng/free.c > > > +++ b/src/malloc/mallocng/free.c > > > @@ -119,7 +119,13 @@ void free(void *p) > > > if (((uintptr_t)(start-1) ^ (uintptr_t)end) >= 2*PGSZ && g->last_idx) { > > > unsigned char *base = start + (-(uintptr_t)start & (PGSZ-1)); > > > size_t len = (end-base) & -PGSZ; > > > - if (len) madvise(base, len, MADV_FREE); > > > + if (len) { > > > + // madvise(..., MADV_FREE) returns -EINVAL on old kernels > > > + // POSIX.1-202x requires free() to not modify errno on success > > > + int e = errno; > > > + madvise(base, len, MADV_FREE); > > > + errno = e; > > > + } > > > } > > > > I think we should save the errno early and make sure its restored on > > exit of the function. you should also include . I suggest > > something like: > > > > diff --git a/src/malloc/mallocng/free.c b/src/malloc/mallocng/free.c > > index 40745f97..77bed88b 100644 > > --- a/src/malloc/mallocng/free.c > > +++ b/src/malloc/mallocng/free.c > > @@ -1,6 +1,7 @@ > > #define _BSD_SOURCE > > #include > > #include > > +#include > > > > #include "meta.h" > > > > @@ -102,6 +103,7 @@ void free(void *p) > > { > > if (!p) return; > > > > + int orig_errno = errno; > > This is much costlier. It puts the TLS access (faulting and emulating > on old MIPS) in the path that runs on every call. I didn't think about that. The original suggestion is better then. Thanks! -nc > > Rich