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-boun...@qt-project.org> im Auftrag von Giuseppe D'Angelo via Development <development@qt-project.org> Gesendet: Donnerstag, 9. Juni 2022 11:36 An: development@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.dang...@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 _______________________________________________ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development