[fpc-pascal] Re: RE : RE : RE : Assigning value to ftVariantdatatype& varbytes-still stuck

[Reinier Olislagers]

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!