RE : RE : [fpc-pascal] Re: RE : RE : RE : Assigning value to ftVariantdatatype varbytes-still stuck
I do get a segfault in the last test when assigning to ftWideString256. When tracing the code it crashes when moving the size+1 chracters to the database. This fixed length moving is fundamentally wrong. The string is only 20 or so chars long and TCustomBufDataset.SetFieldData tries to do a move of the full 514 bytes (256+1)*2. This asking for trouble. The source of the data is here a temporary WideString created by casting a string, so probably on the heap. Reading 514 bytes when only 40 odd where allocated is not always working I'll try to fix that one also. I'd appreciate that a lot! Uploaded a patch to http://bugs.freepascal.org/view.php?id=19930. Initially, because of the crash, I wanted to submit another bug report but the more I dig into this, the more I feel the definition of SetFieldData/GetFieldData without a length/size parameter and strings passed as pchar is causing all kind of problems: 1) TStringField.SetAsString copies the string to a buffer with size dsMaxStringSize so that datasets that don't figure out the original length of the string can simply copy the full Field.TDatasize (TBufDataset, TDbf, TMemDataset,...). TWideStringField.SetAsString didn't which caused the crash when the string is shorter than Field.Size. TCustomSqliteDataset uses a StrNew(PChar(Buffer)); to get the length of the string but fails sometimes (see 4). 2) dsMaxStringSize isn't enforced for TStringField. Defining a Field.size dsMaxStringSize causes a crash in TStringField.SetAsString. I haven't raised an issue on this, yet. 3) some of the speed advantage of memory based datasets is offset by moving full Field.Tdatasize bytes in both set and get fielddata 4) pascal strings can contain #0 characters in the string. When converting to pchar part of these strings is lost. What I propose is: 1 to create overloaded versions of SetFieldData and GetFieldData that include a length parameter, to change TStringField.SetAsString and TWideStringField.SetAsWideString to use these versions and to migrate the different datasets to use these new versions. This way existing (user) code using SetFieldData/GetFieldData will still work (and the user still being responsible for buffer overruns...) and datasets can correctly save and retrieve strings while improving performance. 2 change the bufdataset internal storage for ftstring and ftwidestring fields to include the string length so that stored strings can be retrieved in full when containing #0 characters and without having to copy systematically datasize bytes. However this will introduce an incompatible binary format for TFpcBinaryDatapacketReader. Having a second FpcBinaryIdent would allow for both formats to co-habitate. If need be the discussion can be moved to fpc-devel. Ludo PS: no misunderstanding. I'm volunteering to make these changes;) ___ fpc-pascal maillist - fpc-pascal@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Re: [fpc-pascal] Re: RE : RE : RE : Assigning value to ftVariantdatatype varbytes-still stuck
On Wednesday 10 August 2011 11:51:41 Ludo Brands wrote: What I propose is: 1 to create overloaded versions of SetFieldData and GetFieldData that include a length parameter, to change TStringField.SetAsString and TWideStringField.SetAsWideString to use these versions and to migrate the different datasets to use these new versions. This way existing (user) code using SetFieldData/GetFieldData will still work (and the user still being responsible for buffer overruns...) and datasets can correctly save and retrieve strings while improving performance. 2 change the bufdataset internal storage for ftstring and ftwidestring fields to include the string length so that stored strings can be retrieved in full when containing #0 characters and without having to copy systematically datasize bytes. However this will introduce an incompatible binary format for TFpcBinaryDatapacketReader. Having a second FpcBinaryIdent would allow for both formats to co-habitate. MSEgui version of tbufdataset uses UnicodeString in recordbuffer for string fields, see file lib/common/db/msebufdataset.pas. Martin ___ fpc-pascal maillist - fpc-pascal@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Re: RE : RE : [fpc-pascal] Re: RE : RE : RE : Assigning value to ftVariantdatatype varbytes-still stuck
On 10-8-2011 11:51, Ludo Brands wrote: I do get a segfault in the last test when assigning to ftWideString256. When tracing the code it crashes when moving the size+1 chracters to the database. This fixed length moving is fundamentally wrong. The string is only 20 or so chars long and TCustomBufDataset.SetFieldData tries to do a move of the full 514 bytes (256+1)*2. This asking for trouble. The source of the data is here a temporary WideString created by casting a string, so probably on the heap. Reading 514 bytes when only 40 odd where allocated is not always working I'll try to fix that one also. I'd appreciate that a lot! Uploaded a patch to http://bugs.freepascal.org/view.php?id=19930. Initially, because of the crash, I wanted to submit another bug report but the more I dig into this, the more I feel the definition of SetFieldData/GetFieldData without a length/size parameter and strings passed as pchar is causing all kind of problems: 1) TStringField.SetAsString copies the string to a buffer with size dsMaxStringSize so that datasets that don't figure out the original length of the string can simply copy the full Field.TDatasize (TBufDataset, TDbf, TMemDataset,...). TWideStringField.SetAsString didn't which caused the crash when the string is shorter than Field.Size. TCustomSqliteDataset uses a StrNew(PChar(Buffer)); to get the length of the string but fails sometimes (see 4). 2) dsMaxStringSize isn't enforced for TStringField. Defining a Field.size dsMaxStringSize causes a crash in TStringField.SetAsString. I haven't raised an issue on this, yet. 3) some of the speed advantage of memory based datasets is offset by moving full Field.Tdatasize bytes in both set and get fielddata 4) pascal strings can contain #0 characters in the string. When converting to pchar part of these strings is lost. What I propose is: 1 to create overloaded versions of SetFieldData and GetFieldData that include a length parameter, to change TStringField.SetAsString and TWideStringField.SetAsWideString to use these versions and to migrate the different datasets to use these new versions. This way existing (user) code using SetFieldData/GetFieldData will still work (and the user still being responsible for buffer overruns...) and datasets can correctly save and retrieve strings while improving performance. 2 change the bufdataset internal storage for ftstring and ftwidestring fields to include the string length so that stored strings can be retrieved in full when containing #0 characters and without having to copy systematically datasize bytes. However this will introduce an incompatible binary format for TFpcBinaryDatapacketReader. Having a second FpcBinaryIdent would allow for both formats to co-habitate. If need be the discussion can be moved to fpc-devel. Ludo PS: no misunderstanding. I'm volunteering to make these changes;) I'd be happy to help with testing at least! ___ fpc-pascal maillist - fpc-pascal@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-pascal
