Re: [REVIEW][3-5] fix STL assertion in vcl graphite code
Petr Mladek píše v Út 06. 03. 2012 v 12:23 +0100: Michael Stahl píše v St 29. 02. 2012 v 23:08 +0100: this patch fixes a STL assertion in GraphiteLayout::expandOrCondense; since i don't know how the multitude of arrays in there are supposed to be used, i thought it's probably a good idea to check the size of the array that will be indexed in the next line, as opposed to one that isn't mentioned in the block... in case this is the right fix, it should be applied to libreoffice-3-5, and possibly libreoffice-3-4 as well (haven't checked). http://cgit.freedesktop.org/libreoffice/core/commit/?id=d066f7e4afb3c9e395932ba7bf8715ad0770bcdd I do not understand the code either. I just wonder if it would make sense to check for: while (++nCharIndex - mnMinCharPos static_castint(mvChar2BaseGlyph.size())) As the array is accessed as mvChar2BaseGlyph[nCharIndex-mnMinCharPos]. Otherwise, the cycle might end too early. Another conservative solution would be to combine it with the original check: while (++nCharIndex static_castint(mvGlyph2Char.size()) nCharIndex - mnMinCharPos static_castint(mvChar2BaseGlyph.size())) Any other opinion here, please? Best Regards, Petr ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW][3-5] fix STL assertion in vcl graphite code
Dear Petr, Thank you for the reminder of this issue. My apologies for not getting to it earlier. I do not understand the code either. I just wonder if it would make sense to check for: while (++nCharIndex - mnMinCharPos static_castint(mvChar2BaseGlyph.size())) As the array is accessed as mvChar2BaseGlyph[nCharIndex-mnMinCharPos]. Otherwise, the cycle might end too early. Another conservative solution would be to combine it with the original check: while (++nCharIndex static_castint(mvGlyph2Char.size()) nCharIndex - mnMinCharPos static_castint(mvChar2BaseGlyph.size())) Any other opinion here, please? I would go with the first of these. It makes no sense to compare a charIndex against a glyph-char mapping array, whose size is the number of glyphs, not characters. I think it is simply a bug. Notice that I am not claiming authoritative understanding of this code since it was written by Keith, who unfortunately is no longer with us and so cannot comment. I am hoping that when I eventually get time to work on this code, and particularly the mac port, I can refactor the code and gain the necessary authoritative confidence. Yours, Martin ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW][3-5] fix STL assertion in vcl graphite code
Michael Stahl píše v St 29. 02. 2012 v 23:08 +0100: this patch fixes a STL assertion in GraphiteLayout::expandOrCondense; since i don't know how the multitude of arrays in there are supposed to be used, i thought it's probably a good idea to check the size of the array that will be indexed in the next line, as opposed to one that isn't mentioned in the block... in case this is the right fix, it should be applied to libreoffice-3-5, and possibly libreoffice-3-4 as well (haven't checked). http://cgit.freedesktop.org/libreoffice/core/commit/?id=d066f7e4afb3c9e395932ba7bf8715ad0770bcdd I do not understand the code either. I just wonder if it would make sense to check for: while (++nCharIndex - mnMinCharPos static_castint(mvChar2BaseGlyph.size())) As the array is accessed as mvChar2BaseGlyph[nCharIndex-mnMinCharPos]. Otherwise, the cycle might end too early. Another conservative solution would be to combine it with the original check: while (++nCharIndex static_castint(mvGlyph2Char.size()) nCharIndex - mnMinCharPos static_castint(mvChar2BaseGlyph.size())) Best Regards, Petr ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[REVIEW][3-5] fix STL assertion in vcl graphite code
this patch fixes a STL assertion in GraphiteLayout::expandOrCondense; since i don't know how the multitude of arrays in there are supposed to be used, i thought it's probably a good idea to check the size of the array that will be indexed in the next line, as opposed to one that isn't mentioned in the block... in case this is the right fix, it should be applied to libreoffice-3-5, and possibly libreoffice-3-4 as well (haven't checked). http://cgit.freedesktop.org/libreoffice/core/commit/?id=d066f7e4afb3c9e395932ba7bf8715ad0770bcdd ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
