About use of sal_IntPtr for FontList::GetSizeAry, aStdSizeAry and mpSizeAry (svtools)

[Julien Nabet]

Hello,

IMHO the use of sal_IntPtr in FontList class is wrong.

It's been there since 43135665bf9093c52f424069bcf83d50a93bdc0c:

author    Fridrich Štrba <fridrich.strba at bluewin.ch> 2013-06-10 12:49:33 
+0200
committer    Fridrich Štrba <fridrich.strba at bluewin.ch> 2013-06-10 
14:03:34 +0200
commit    43135665bf9093c52f424069bcf83d50a93bdc0c (patch)
tree    079359e37353f3a936656a9d3aeff2e3577cc353
parent    378b2522b40004ca5f5b6de0b4eda0ac13d4153d (diff)
mingw64: use integers able to contain a size in svtools
Change-Id: Id5505f75a2331be682b74d085a7959fc4bf07df8

Let's take a look at the 2 vars:

1) aStdSizeAry

it's a static array containing values from 0 to 960.

(see 
https://opengrok.libreoffice.org/xref/core/svtools/source/control/ctrltool.cxx?r=770ca016#38)

2) mpSizeAry

git grep -n mpSizeAry in core repo gives:
include/svtools/ctrltool.hxx:149: mpSizeAry;
svtools/source/control/ctrltool.cxx:749:    mpSizeAry.reset();
svtools/source/control/ctrltool.cxx:772:    mpSizeAry.reset(new 
sal_IntPtr[nDevSizeCount+1] );
svtools/source/control/ctrltool.cxx:779: mpSizeAry[nRealCount] = nOldHeight;
svtools/source/control/ctrltool.cxx:783:    mpSizeAry[nRealCount] = 0;
svtools/source/control/ctrltool.cxx:786:    return mpSizeAry.get();

so the elements are only initialized thanks to "nOldHeight" which itself 
is declared as a long and is initialized with "aSize.Height()"

(see 
https://opengrok.libreoffice.org/xref/core/svtools/source/control/ctrltool.cxx?r=770ca016#778).

Indeed, according to include/tools/gen.hxx, "Height()" returns a long, 
so "nOldHeight" is ok.

(see 
https://opengrok.libreoffice.org/xref/core/include/tools/gen.hxx?r=32090b01#189)


I know there has been some problem about types of attributes of Size, 
there's even been an attempt to use sal_Int32 but reverted with 
d36f7c5bd2115fcdd26ba8ff7b6a0446dea70bd4

"Revert "long->sal_Int32 in tools/gen.hxx"

/This reverts commit //8bc951daf79decbd8a599a409c6d33c5456710e0 
<https://cgit.freedesktop.org/libreoffice/core/commit/?id=8bc951daf79decbd8a599a409c6d33c5456710e0>//. 
As discussed at 
<https://lists.freedesktop.org/archives/libreoffice/2018-April/079955.html> 
"long->sal_Int32 in tools/gen.hxx", that commit caused lots of problems 
with signed integer overflow, and the original plan was to redo it to 
consistently use sal_Int64 instead of sal_Int32. 
<https://gerrit.libreoffice.org/#/c/52471/> "sal_Int32->sal_Int64 in 
tools/gen.hxx" tried that. However, it failed miserably on Windows, 
causing odd failures like not writing out Pictures/*.svm streams out 
into .odp during CppunitTest_sd_export_ooxml2. So the next best approach 
is to just revert the original commit, at least for now. Includes revert 
of follow-up //8c50aff2175e85c54957d98ce32af40a3a87e168 
<https://cgit.freedesktop.org/libreoffice/core/commit/?id=8c50aff2175e85c54957d98ce32af40a3a87e168>//"Fix 
Library_vclplug_qt5"."/

Anyway for the moment do you think we could change the use of sal_IntPtr 
for these quoted elements into long types?

Julien

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/libreoffice/attachments/20200910/aa166697/attachment.htm>