Re: [boost] [context] Implementation comments

Subject: Re: [boost] [context] Implementation comments
From: vicente.botet (vicente.botet_at_[hidden])
Date: 2011-01-17 07:36:57

----- Original Message ----- 
From: "Oliver Kowalke" <k-oli_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Thursday, December 16, 2010 9:06 PM
Subject: [boost] [context] new version - support for Win64

> I've uploaded boost.context-0.3.0.zip to boost vault.
> 
> The new version supports the context-/stack- swapping provided by the OS 
> (ucontext ABI on UNIX, Fiber ABI on Windows) as well a fast context 
> swapping (using assembler).
 
I have taken a look at the implementation and I would preffer an implementation of Boost.Context much more in the spirit of a simple C++ wrapper for a low level service. IMO the cost of using Boost.Context, repect to the direct use of for example ucontext/fcontext,  should be reduced to the minimum as it is a low level library.

* Pimpl: The use of Pimpl idiom for a low level sevice seems to me overdesigned.
* Movable: Why do we need to make context movable? Couldn't unique_ptr<context> be used to move contexts.
* Stack and protection: I think that where the stack memory is located and whether it is protected or not are concerns that are  independent from the context library. I would add to the context creation two parameter: the pointer to the stack and the stack size, and let the user the management of whether this memory is protected or not. The following code should IMO be on the user side

                std::size_t pages( helper::calc_pages( stacksize, helper::pagesize) );
                if ( with_stack_protection == prot) pages += 1; // add guard page
                ctx.uc_stack.ss_size = pages * helper::pagesize;

                int fd( ::open("/dev/zero", O_RDONLY) );
                if ( -1 == fd) throw std::bad_alloc();
                ctx.uc_stack.ss_sp = static_cast< char * >(
                        ::mmap( 0, ctx.uc_stack.ss_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0) );
                ::close( fd);
                if ( ! ctx.uc_stack.ss_sp) throw std::bad_alloc();
                
                if ( with_stack_protection == prot)
                        if ( 0 != ::mprotect( ctx.uc_stack.ss_sp, helper::pagesize, PROT_NONE) )
                                throw std::bad_alloc();

Of course the user can define his own class that gets mmap with protection or not and unmap it on destruction

Maybe the library should provide the thin layer over ucontext (i.e. the fcontext implementation detail functions).

We could have in addition some kind of static_context that have the stack size as template parameter and contains itself the stack memory area.

Best,
Vicente