Hello Patrick, On 11 Jul 2014, at 15:27, Patrick Ohly <patrick.o...@intel.com> wrote:
> I'm currently investigating why a comparison of two PHOTO fields of
> different length returns "field equal". PHOTO is defined as:
>
> <field name="PHOTO" type="string" compare="conflict" merge="fillempty"/>
My first question would be why PHOTO was defined "string" here, not "blob"?
> I think the specific situation is that the two values contain a null
> byte somewhere in the middle, and the part before that is equal.
>
> I think the following code does the comparison, doesn't it?
>
> sInt16 TStringField::compareWith(TItemField &aItemField, bool
> aCaseInsensitive)
> {
> sInt16 result;
> PULLFROMPROXY;
> if (aItemField.isBasedOn(fty_string)) {
> TStringField *sfP = static_cast<TStringField *>(&aItemField);
> #ifdef STREAMFIELD_SUPPORT
> sfP->pullFromProxy(); // make sure we have all chars
> #endif
> // direct compare possible, return strcmp
> if (aCaseInsensitive)
> result=strucmp(fString.c_str(),sfP->fString.c_str());
> else
> result=strcmp(fString.c_str(),sfP->fString.c_str());
> }
> ...
Yes, but this was written for text strings. I admit it is very c-ish style, but
correct enough for text strings.
IMHO The real bug is that TBlobField does not override compareWith.
TBlobField's compareWith could ignore aCaseInsensitive and use memcmp, for the
case of actually comparing two blobs, and fall back to TStringField for
comparison with other types.
> We have std::string as value and therefore can store null bytes as part
> of the data, but the actual comparison falls back to C-string
> operations, which only work for null bytes at the end.
There's to much C strings throughout the entire project, so for text, C string
semantics are assumed pretty much everywhere.
TBlobField however makes use of the true binary byte string capability of
std::string exactly this way.
> I think the strcmp() needs to be replaced with something that also looks
> at the rest of the string if no difference is found. Agreed?
A fully std::string compatible comparison would be nicer here, and fix the
problem.
> Or do we need a real "binary" type?
We do have it :-) It's called BLOB :-)
> Using string type for PHOTO also has
> the risk that during a merge operation, the two values will get
> concatenated (thus breaking the images) unless some custom merge script
> resolves the conflict differently first.
Probably others, because TStringField was certainly never designed to carry
binary data, altough it can do it to some extent.
Best Regards,
Lukas
signature.asc
Description: Message signed with OpenPGP using GPGMail
_______________________________________________ os-libsynthesis mailing list os-libsynthesis@synthesis.ch http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
