Changes that we made to our local copy
Posted: Tue Aug 30, 2016 1:40 pm
Hello,
We just downloaded the latest version and merged it with our own copy here. In this copy, we have some changes when compared to your public version and we believe those changes could benefit all users of the suite. Here is a detailed summary, file by file
BIFF_ReadII5.pas
Inside TXLSReadII.LoadFromStream, there is a big try.except block. Inside the except part, we added this, right before on E: Exception :
This is because our handler for the OnProgress event may call the Abort function and we did not want this exception to be masked by the general handler. This also means that EAbort is swallowed silently up the chain of exception handlers.
Inside TXLSReadII.RREC_FORMULA, there is this call
If FLastArray is already allocated, this means a memory leak. To avoid this, we added those lines right before the call to GetMem:
xc12DataStyleSheet5.pas
When doing performance analysis, we found that TXc12CellStyles.Find would be very slow if there are lots of styles. To improve this, we made this method use a hashmap (TDictionary<string, TXc12CellStyle>) to find its elements. This allowed great performance gains but this also means that it is only compatible with very recent versions of Delphi.
As the changes are a bit complex to explain, I won't post them here, but if you are interested, I can send you a diff file.
XLSReadXLSX5.pas
Inside TXLSReadXLSX.ReadStyleCellStyles, there is this code:
In the previous version we were using, only the commented out line of code was present. But with the current code, we can't open most of our XLSX files as it triggers a 'Duplicates not allowed' exception.
Commenting out the two bottom lines and uncommenting the first one allowed us to be able to load the files again. If need be, I can provide such a file.
XLSSheetData5.pas
Inside TXLSWorksheet.GetAsVariant, you added the else part where Result is set to zero. Having this else part is a good thing, we had already made the change locally. However we decided to return Null in that case instead of zero, thus allowing to distinguish between a cell with value 0 and a a cell that was not found.
xpgPXML.pas
On various projects, we have activated overflow checks and we get an error inside TXpgReadXML.QNameHashB because this line overflows:
Now, we understand the fact that it should overflow and that it is not harmful here, even desired. But to avoid receiving an error, we surrounded the whole procedure with this:
BIFF_ReadII5.pas, XLSMMU5.pas, xpgPXML.pas, xpgPXMLUtils.pas
We replaced all casts from pointer to Integer or Longword to casts to NativeInt or NativeUInt respectively so that we are sure that no arithmetics ever overflow when used under x64 targets.
This is a strict policy that we have here where pointers should never be cast to anything other than NativeInt or NativeUInt to avoid nasty surprises in hard to track locations. Granted, in some instances, it is a bit of an overkill, but we want to be on the safe side with this.
We hope that some of these changes will make it into a future release, do not hesitate to ask us if you have questions.
Regards
We just downloaded the latest version and merged it with our own copy here. In this copy, we have some changes when compared to your public version and we believe those changes could benefit all users of the suite. Here is a detailed summary, file by file
BIFF_ReadII5.pas
Inside TXLSReadII.LoadFromStream, there is a big try.except block. Inside the except part, we added this, right before on E: Exception :
Code: Select all
on EAbort do
raise;
Inside TXLSReadII.RREC_FORMULA, there is this call
Code: Select all
GetMem(FLastARRAY, Header.Length);
Code: Select all
if FLastARRAY <> Nil then
FreeMem(FLastARRAY);
When doing performance analysis, we found that TXc12CellStyles.Find would be very slow if there are lots of styles. To improve this, we made this method use a hashmap (TDictionary<string, TXc12CellStyle>) to find its elements. This allowed great performance gains but this also means that it is only compatible with very recent versions of Delphi.
As the changes are a bit complex to explain, I won't post them here, but if you are interested, I can send you a diff file.
XLSReadXLSX5.pas
Inside TXLSReadXLSX.ReadStyleCellStyles, there is this code:
Code: Select all
// Style := FManager.StyleSheet.Styles.AddOrGet(S.Name);
Style := FManager.StyleSheet.Styles.Add;
Style.Name := S.Name;
Commenting out the two bottom lines and uncommenting the first one allowed us to be able to load the files again. If need be, I can provide such a file.
XLSSheetData5.pas
Inside TXLSWorksheet.GetAsVariant, you added the else part where Result is set to zero. Having this else part is a good thing, we had already made the change locally. However we decided to return Null in that case instead of zero, thus allowing to distinguish between a cell with value 0 and a a cell that was not found.
xpgPXML.pas
On various projects, we have activated overflow checks and we get an error inside TXpgReadXML.QNameHashB because this line overflows:
Code: Select all
a := a * 378551Code: Select all
{$IFOPT Q+}
{$OVERFLOWCHECKS OFF}
{$DEFINE OVERFLOWCHECKS_WAS_ON}
{$ENDIF}
function TXpgReadXML.QNameHashB: longword;
...
{$IFDEF OVERFLOWCHECKS_WAS_ON}
{$OVERFLOWCHECKS ON}
{$ENDIF}
We replaced all casts from pointer to Integer or Longword to casts to NativeInt or NativeUInt respectively so that we are sure that no arithmetics ever overflow when used under x64 targets.
This is a strict policy that we have here where pointers should never be cast to anything other than NativeInt or NativeUInt to avoid nasty surprises in hard to track locations. Granted, in some instances, it is a bit of an overkill, but we want to be on the safe side with this.
We hope that some of these changes will make it into a future release, do not hesitate to ask us if you have questions.
Regards