Re: [Libreoffice-commits] core.git: initialize new member variables

Sent from my iPad, please excuse any misspellings 

> On 12 Jan 2016, at 12:05, Chris Sherlock <chris.sherlock79@gmail.com> wrote:
>
> Hi all,
>
> Unfortunately that was the case - I definitely went down the wrong path with that merge.
>
> I've reverted it now - the commit can be found on 
> http://cgit.freedesktop.org/libreoffice/core/commit/?id=b4b0cc2a5eef42434444e51fda4a13fc48183aa0
>
> I need to check that UBSan tool more regularly.
>
> But I definitely have to put my hand up for causing these errors. Apologies for this, I will do 
> my level best not to let this occur again.
>
> When I'm back home I've been meaning to send a post to the list summarising how font handling 
> works and some ideas and questions around the code.
Even better would be to make a wiki page :-)

rgds
jan i
> Thanks all (especially Caolon and Stephan)!
>
> Chris
>
> > > On 12 Jan 2016, at 7:20 PM, Stephan Bergmann <sbergman@redhat.com> wrote:
> > >
> > > On 01/11/2016 06:29 PM, Caolán McNamara wrote:
> > > commit c93a43c0c9113bc1c787fe6b3c5e58a8bfd80be0
> > > Author: Caolán McNamara <caolanm@redhat.com>
> > > Date:   Mon Jan 11 17:08:39 2016 +0000
> > >
> > >    initialize new member variables
> > >
> > >    Change-Id: I3839bc134b337ccb7cfdb2ee70524e4721c8f83c
> > >
> > > diff --git a/vcl/source/font/fontattributes.cxx b/vcl/source/font/fontattributes.cxx
> > > index be3ab68..49060f0 100644
> > > --- a/vcl/source/font/fontattributes.cxx
> > > +++ b/vcl/source/font/fontattributes.cxx
> > > @@ -93,7 +93,18 @@ bool FontAttributes::CompareDeviceIndependentFontAttributes(const 
> > > FontAttributes
> > > }
> > >
> > > FontAttributes::FontAttributes()
> > > -    : mnWidth ( 0 )
> > > +    : meWeight(WEIGHT_DONTKNOW)
> > > +    , meItalic(ITALIC_DONTKNOW)
> > > +    , meFamily(FAMILY_DONTKNOW)
> > > +    , mePitch(PITCH_DONTKNOW)
> > > +    , meWidthType(WIDTH_DONTKNOW)
> > > +    , mbSymbolFlag(false)
> > > +    , mnQuality(0)
> > > +    , mbOrientation(false)
> > > +    , mbDevice(false)
> > > +    , mbSubsettable(false)
> > > +    , mbEmbeddable(false)
> > > +    , mnWidth ( 0 )
> > >     , mnOrientation( 0 )
> > >     , mnAscent( 0 )
> > >     , mnDescent( 0 )
> > [...]
> >
> > Tools like UBSan or Valgrind's memcheck started to complain about reads of uninitialized members 
> > after a series of commits that merged font-related structs in VCL together.  What looked fishy 
> > is that those members had not been default-initialized in the original structs, the number of 
> > setter calls had not changed, and yet the code started to grow uninitialized reads of the 
> > members.  Without a clear understanding of what was going on, we were reluctant to blindly 
> > default-initialize those members now, as it might hide problems (at least from the eyes of UBSan 
> > and Valgrind) instead of fixing them.
> >
> > Now, as I understand, Chris is going to revert those merges, as he came to the conclusion that 
> > they went in the wrong direction after all, and I assume that will obsolete the above 
> > potentially problematic commit anyway.
> _______________________________________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice