Changes that we made to our local copy

Questions and answers on how to use XLSReadWriteII 5.
Post Reply
actint
Posts: 5
Joined: Thu Jan 03, 2013 1:06 pm

Changes that we made to our local copy

Post by actint »

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 :

Code: Select all

      on EAbort do
        raise;
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

Code: Select all

        GetMem(FLastARRAY, Header.Length);
If FLastArray is already allocated, this means a memory leak. To avoid this, we added those lines right before the call to GetMem:

Code: Select all

        if FLastARRAY <> Nil then
          FreeMem(FLastARRAY);
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:

Code: Select all

//     Style := FManager.StyleSheet.Styles.AddOrGet(S.Name);

     Style := FManager.StyleSheet.Styles.Add;
     Style.Name := S.Name;
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:

Code: Select all

a := a * 378551
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:

Code: Select all

{$IFOPT Q+}
{$OVERFLOWCHECKS OFF}
{$DEFINE OVERFLOWCHECKS_WAS_ON}
{$ENDIF}
function TXpgReadXML.QNameHashB: longword;
...
{$IFDEF OVERFLOWCHECKS_WAS_ON}
{$OVERFLOWCHECKS ON}
{$ENDIF}
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
larsa
Site Admin
Posts: 926
Joined: Mon Jun 27, 2005 9:30 pm

Re: Changes that we made to our local copy

Post by larsa »

Hello

Thank you for your suggestions. I will implement most of them to the next update. If you have problems with some files (as you mention in #XLSReadXLSX5.pas), please send it to me.
Lars Arvidsson, Axolot Data
actint
Posts: 5
Joined: Thu Jan 03, 2013 1:06 pm

Re: Changes that we made to our local copy

Post by actint »

larsa
Site Admin
Posts: 926
Joined: Mon Jun 27, 2005 9:30 pm

Re: Changes that we made to our local copy

Post by larsa »

Hello

I can't find any problems with your file. And I can't see where in the code a "Duplicates not allowed" exception can be raised. This exception is however generated by classes in Generics.Collections, such as TDictionary. Conclusion: your "fix" to speed up searching of styles is causing the exception because TDictionary don't allows duplicates and there can be duplicate style names. Faster searching for styles requires another type of solution. I will put this on the todo list for the next version.
Lars Arvidsson, Axolot Data
Post Reply