Patch v4: Fix KEncodingProber crashiness

Jeff Mitchell mitchell at kde.org
Sat Mar 21 13:21:12 GMT 2009


Peter Oberndorfer wrote:
> But what if the current prober has not detected anything?
> Then the current confidece in the prober is still 0.0
> 
> The intention of the code is IMO when a unicode BOM is found
> a confidence of 0.99 is returned.
> 
> At least i think it did this until 845343

Unsure...I haven't looked at the history of the code, just how it is
now.  Regardless, in today's code, the variable was a total do-nothing,
as the confidence in the prober is always what is returned.

>>> 5)
>>> I think the prober is leaked on setProberType()?
>> Yes, it is.  Fixed in the attached v3 patch.
>>
> 
> I think a part of this patch is already applied now 942108 ?

Yes, you're right.  I wish Wang had not done this, so that the finalized
patch could be committed in one SVN revision.

>> +        if (prober)
>> +            delete prober;
>> +
> 
> Why is the if needed?
> it is legal to delete a 0 pointer.
> The destructor above does this already.

It's not.  I can fix it if you care (attached).

>> +    QString encoding;
> 
> Why is this a QString?
> it is only ever assigned static char strings.
> and then it is only tested once with isEmpty but never used anywhere else?
> I think it should somehow be returned in encodingNameByteArray
> when unicodeTest() found something.

Better idea: remove it entirely, as it's basically a no-op.  In attached.

>> +//DEPRECATED, do *not* use
>>  const char* KEncodingProber::encodingName() const
>>  {
>> +    return 0;
> 
> Shouldn't we return a strdup'ed text here?
> It leaks memory but still works.
> 
> something like (warning untested!):
>   return strdup(encodingNameByteArray().constData());

I don't really care.  Thiago's suggestion was to simply have it return
nothing and create the QByteArray version.

> or at least fix katebuffer.cpp to not call the old function?

Yes, probably a better idea.

Attached is a v4 patch (against current SVN) fixing these issues
(including katebuffer.cpp).

--Jeff
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fixkencodingprober_v4.patch
Type: text/x-diff
Size: 4451 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20090321/85db176c/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 260 bytes
Desc: OpenPGP digital signature
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20090321/85db176c/attachment.sig>


More information about the kde-core-devel mailing list