[PATCH 1/3] copyfd: reinstate proper guard around munmap()

Johannes Schindelin Johannes.Schindelin at gmx.de
Fri Jul 14 20:14:07 UTC 2017


Hi Denys,

On Fri, 14 Jul 2017, Denys Vlasenko wrote:

> On Fri, Jul 14, 2017 at 4:11 PM, Johannes Schindelin
> <johannes.schindelin at gmx.de> wrote:
> > In 4c1392296 (Introduce FEATURE_COPYBUF_KB, 2007-12-02), a feature was
> > introduced where a large stack was allocated via mmap(), and
> > consequently released via munmap(). Since this is overkill for small
> > stacks, the mmap()/munmap() code was guarded inside an #if
> > CONFIG_FEATURE_COPYBUF_KB > 4 ... #endif guard.
> >
> > In pure Win32 API, there is no support to abuse mmap() to allocate a
> > stack that way, therefore the code is simply disabled in busybox-w32.
> >
> > But 8d75d794e (libbb: use sendfile() to copy data between file
> > descriptors, 2014-11-27) removed that guard around the munmap() call.
> > That was most likely a mistake, as the corresponding mmap() call is
> > *still* inside an equivalent #if ... #endif guard.
> >
> > Let's revert the mistaken change.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin at gmx.de>
> > ---
> >  libbb/copyfd.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/libbb/copyfd.c b/libbb/copyfd.c
> > index 7e3531903..eedf03790 100644
> > --- a/libbb/copyfd.c
> > +++ b/libbb/copyfd.c
> > @@ -119,8 +119,10 @@ static off_t bb_full_fd_action(int src_fd, int dst_fd, off_t size)
> >         }
> >   out:
> >
> > +#if CONFIG_FEATURE_COPYBUF_KB > 4
> >         if (buffer_size > 4 * 1024)
> >                 munmap(buffer, buffer_size);
> > +#endif
> 
> If CONFIG_FEATURE_COPYBUF_KB <= 4, then buffer_size is a constant < 4k
> and compiler will eliminate the code anyway.

That would be correct only when optimizing.

In a debug build, in a setup without mmap()/munmap(), the current code
simply fails to build.

My patch fixes that build failure.

Ciao,
Johannes


More information about the busybox mailing list