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