Re: [musl] [PATCH v2] MT fork

[Rich Felker <dalias@libc.org>]

On Wed, Nov 11, 2020 at 12:25:00PM +0100, Szabolcs Nagy wrote:
> * Alexey Izbyshev <izbyshev@ispras.ru> [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<n; i+=l) {
> > > +        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