[Development] Asking for a FF exception for ICU based QStringConverter

[Fabian Kosmale]

Hi,

with the naive approach, old code that used to fail would leak one ICU converter per constructed QStringConverter.
I've outlined an approach in the patch which would however avoid that issue (old code would keep failing to
create a converter).

Only adding support in the XML classes would not be enough in general; it might  help with
the bug report referenced in the patch, though.

QTextStream has indeed no way to change the codec so far, because setEncoding only takes an enum,
not a name.  If we deem that to be a glaring API deficiency (which cannot be delayed until Qt 6.5), then
yes, that would also require an additional patch. In we deem that too much for post-FF, maybe deferring the
change to 6.5 is indeed the best option.

QSettings doesn't have a way to change the codec at all as far as I can tell, and is UTF-8 only, so I'm not sure how
relevant that is for the discussion.

Regards,
Fabian

________________________________________
Von: Development <development-bounces at qt-project.org> im Auftrag von Giuseppe D'Angelo via Development <development at qt-project.org>
Gesendet: Donnerstag, 9. Juni 2022 11:36
An: development at qt-project.org
Betreff: Re: [Development] Asking for a FF exception for ICU based QStringConverter

Il 09/06/22 01:48, Thiago Macieira ha scritto:
>
> d) Because if this, code using QStringConverter with non-builtin encodings
> will leak resources unless it's recompiled for 6.4. No source changes are
> necessary.
>
> I am saying that (d) is an acceptable situation because of (a) and (b), and in
> spite of (c).

So basically this is a soft ABI break?

Code currently using QStringConverter on a non-UTF encoding is failing
(but not leaking anything). Same code with an upgraded Qt will work, but
will leak memory (how much? once per QStringConverter object? once per
encoding?); a recompilation is needed to stop the leak.


I'm not really sure how much QStringConverter is used _directly_ by
client code, but a random search shows that the number is not zero:

> https://lxr.kde.org/ident?v=kf5-qt5&_i=QStringConverter&_remember=1


I'm also concerned that this won't pass Alpha review without adding more
APIs around. How exactly do I set a QStringEncoder/Decoder with a custom
encoding on top of a QTextStream, QSettings, etc.?


With these two concerns combined, I'm close to -1 this idea.

(I'm perfectly fine with adding a (Qt-private) way for the XML classes
to deal with non-UTF encodings, but that's not sufficient, is it?)


My 2 c,
--
Giuseppe D'Angelo | giuseppe.dangelo at kdab.com | Senior Software Engineer
KDAB (France) S.A.S., a KDAB Group company
Tel. France +33 (0)4 90 84 08 53, http://www.kdab.com
KDAB - The Qt, C++ and OpenGL Experts