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 17235 invoked from network); 11 Nov 2020 14:56:19 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 11 Nov 2020 14:56:19 -0000 Received: (qmail 11976 invoked by uid 550); 11 Nov 2020 14:56:17 -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 11958 invoked from network); 11 Nov 2020 14:56:16 -0000 Date: Wed, 11 Nov 2020 09:56:03 -0500 From: Rich Felker To: Alexey Izbyshev , musl@lists.openwall.com Message-ID: <20201111145603.GK534@brightrain.aerifal.cx> References: <5298816.XTEcGr0bgB@nanabozho> <20201031033117.GH534@brightrain.aerifal.cx> <20201106033616.GX534@brightrain.aerifal.cx> <20201108161215.GE1370092@port70.net> <20201109170729.GA534@brightrain.aerifal.cx> <20201109215901.GG1370092@port70.net> <20201109222320.GC534@brightrain.aerifal.cx> <20201111005216.GH534@brightrain.aerifal.cx> <20201111112500.GI1370092@port70.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201111112500.GI1370092@port70.net> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [PATCH v2] MT fork On Wed, Nov 11, 2020 at 12:25:00PM +0100, Szabolcs Nagy wrote: > * Alexey Izbyshev [2020-11-11 09:35:22 +0300]: > > On 2020-11-11 03:52, Rich Felker wrote: > > > Here's a proposed first patch in series, getting rid of getdelim/stdio > > > usage in ldso. I think that suffices to set the stage for adding > > > __libc_malloc, __libc_free, __libc_calloc, __libc_realloc and having > > > ldso use them. > > > > > if we don't have to replicate a lot of code in ldso then this sounds good. Indeed the ldso part of the patch it just: +#define malloc __libc_malloc +#define calloc __libc_calloc +#define realloc __libc_realloc +#define free __libc_free That's the minimal needed to make it work. Assuming we adopt and keep this I might also remove a bunch of ugly code in dynlink.c that special-cases whether it's running with replaced malloc or not. > > > +static ssize_t read_loop(int fd, void *p, size_t n) > > > +{ > > > + unsigned char *b = p; > > > + for (size_t l, i=0; i > > + l = read(fd, b+i, n-i); > > > + if (l<0) { > > > + if (errno==EINTR) continue; > > This increments `i` by a negative `l`. Thanks Alexey for catching that! > it's worse: l cannot be negative so the error check is ineffective. > > maybe it should be ssize_t? or check == -1 Yes, there are multiple problems and this was the original motivation for using stdio -- not having to write this ugly error-prone code. But it only has to be written and reviewed once so it shouldn't be too bad to get it right. ssize_t should be fine. I mainly did ridiculous stuff trying to be clever with scope of the vars. > > > + if (fd>=0) { > > > + size_t n = 0; > > > + if (!fstat(fd, &st)) n = st.st_size; > > > + sys_path = malloc(n+1); > > > + sys_path[n] = 0; > > `sys_path` can be NULL here. Thanks, I meant to put that in if ((sys_path = malloc(n+1))) or something. Will fix. Rich