Re: [Libreoffice] [REVIEW] Fix for fdo#37226

On Wed, 2011-05-18 at 12:08 +0100, Noel Power wrote:
> On 17/05/11 17:04, Kohei Yoshida wrote:
> > Hi there,
> >
> > The attached patch fixes the crasher reported in the following bug:
> >
> > https://bugs.freedesktop.org/show_bug.cgi?id=37226
> >
> > and also several other usage of standard algorithms which I believe is
> > incorrect, since the existing code passes the result of an evaluation
> > which is always boolean (unless boost::bind overloads those operators to
> > return something else, which I doubt it does).
> I don't know much about boost, have seen the bind stuff peppered here 
> and there in the code and generally liked the look of it, anyway your 
> question prompted me to try and understand more, the following link 
> http://www.boost.org/doc/libs/1_46_1/libs/bind/bind.html#operators seems 
> to indicate that it is the operator is passed to the standard algorithm 
> and not the result ( but maybe I misunderstood, this stuff is still 
> unfamiliar to me )

No you are right; boost::bind indeed overloads those operators for
convenience as I later found out.

> >    There were several cases
> > where such evaluation result was passed to standard algorithms, one of
> > which caused the crash.
> I tried your patch ( but with the bind bits left in ) and it seemed to 
> work find for me ( especially wrt to the operator stuff above ) attached 
> is the patch I used

So, we (Rafael, Norbert, Joseph and I) talked about this last night on
IRC, and one question raised was whether or not boost::bind allows
pointers to data members rather than member functions.  The
documentation clearly mentions of members functions, but is a bit
ambiguous with data members.  In fact, the standard function adapters
that boost::bind wraps seems to only support member functions which made
me believe that the use of bind with data members is not allowed and
that if it works with some compilers it's just by accident and is not
really guaranteed with different compilers.

Having said that, if boost::bind is guaranteed to work with data
members, then I prefer keeping that piece in & prefer your patch over
mine.  Otherwise, I'd rather we don't use it with data members.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc
<kyoshida@novell.com>