Keysmith in kdereview

Johan Ouwerkerk jm.ouwerkerk at gmail.com
Sat Dec 28 14:22:42 GMT 2019


First of all: sorry for the duplicate reply Albert. There was a mixup
with the reply button on my end, unfortunately.

Here goes attempt #2:

> I think it'd be good if you used a QVarLengthArray instead of "char code[m_pinLength];"
>

Yes that is a known wart. There are a few issues/MRs that touch on
this particular point:

 - https://invent.kde.org/kde/keysmith/merge_requests/24 : as a side
effect of refactoring it is taken care of in a proposed MR to fix
issue 2: https://invent.kde.org/kde/keysmith/issues/2
 - https://invent.kde.org/kde/keysmith/issues/6 : we still have to
implement encryption/decryption of secrets and that may also impact
the code that generates tokens
 - https://invent.kde.org/kde/keysmith/issues/9 : we may want to drop
liboath (oath-toolkit) due to its limitations and quirks and
difficulty getting it to build on platforms where it is not packaged
(i.e. Android)

We don't need to use QVarLengthArray or any VLA, really: this is not
performance critical and pre-allocating a suitably sized QByteArray
does the job just as well but in a format that is a bit more natural
for converting into QString later (QString::fromUtf8). But the general
criticism of code like `char code[m_pinLength];` is definitely valid
:)

Regards,

 - Johan




More information about the kde-core-devel mailing list