Re: [Boost-users] [boost] [xint] Boost.XInt formal review

Subject: Re: [Boost-users] [boost] [xint] Boost.XInt formal review
From: Christopher Jefferson (chris_at_[hidden])
Date: 2011-03-02 09:16:38

One brief early comment, to help reviewers (and the author).

The library has a small problem which prevents it compiling with clang (and possibly other compilers). I in no way view this as a serious issue, as getting code to work with many compilers is always a problem.

On lines 85 and 89 of boost/xint/integer.hpp, remove the 'typename'.

After this fix, I briefly used the library and used the documentation. It is indeed a large number library, I wrote some short programs and they worked well.

I congratulate the author on making the easy uses of xint easy. I am sure others will comment on the advanced features of the library, but I am very happy to finally have a library where I can just declare a boost::xint::integer, and then have all normal operations work as I would expect, with no need for boiler-plate code.

I also tried writing some incorrect code, and the error messages are clear and short, which is nice to see.

I agree with the overall design decision chosen for thread-safety, and believe the system correctly balances clarity and efficiency.

From my brief review as a user, I believe this library should be accepted.

One serious (but small) issue which will have to be addressed before the library is accepted.

The 'secure' flag at the moment I believe cannot be trusted to work. Compilers can, and do, optimise out memset if it can prove the memory will not be changed again.

There are various solutions to this, I would probably suggest putting a "secure memset" into boost/detail so it can be used by other libraries, and specialised for different systems.

See Microsoft's discussion of the issue here:

http://msdn.microsoft.com/en-us/library/ms972826.aspx

The short answer is:

on Windows wrap the memset in:

#pragma optimize("", off)
#pragma optimize("", on)

On linux, write a "stupid" memset which uses volatile pointers to force the code to be executed.

I am happy to help clean this up, and produce a general solution, if required.

On 2 Mar 2011, at 10:58, Vladimir Prus wrote:

> Boosters,
> 
> the formal review of the Boost.XInt library, by Chad Nelson, starts now and will
> run through March 11.
> 
> The documentation for the current version is available at:
> 	
> 	http://www.oakcircle.com/xint_docs/
> 
> The source code is available via Subversion at:
> 
> 	http://svn.boost.org/svn/boost/sandbox/xint
> 
> or can be downloaded at:
> 
>    http://www.boostpro.com/vault/index.php?action=downloadfile&filename=xint.zip
> 
> Here are the important points:
> 
> 1. All comments are appreciated. Even if you don't have the time for in-depth study
> of everything, comments on parts of the library are welcome. In particular, if you
> are already using logging in your applications and have specialized requirements,
> you might want to directly check how the proposed library handles them.
> 
> 2. The reviews and all comments should be submitted to the developers list,
> boost_at_[hidden] and the email should have "[xint]" prefix in the subject
> to make sure it's not missed. The deadline for reviews is
> 
>     23:59, March 11, PST, or
>     2:59, March 12, EST, or
>     7:59 March 12, UK time,
>     10:59, March 12 MSK
> 
> 3. Please explicitly state in your review whether the library should be accepted.
> 
> 4. The general review checklist is provided below:
> 
>        - What is your evaluation of the design?
>        - What is your evaluation of the implementation?
>        - What is your evaluation of the documentation?
>        - What is your evaluation of the potential usefulness of the library?
>        - Did you try to use the library?  With what compiler?  Did you have any
>        problems?
>        - How much effort did you put into your evaluation? A glance? A quick
>        reading? In-depth study?
>        - Are you knowledgeable about the problem domain?
> 
> Thanks,
> Volodya
> 
> -- 
> Vladimir Prus
> Mentor Graphics
> [phone]
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost