Sharing my progress (EteSync sync backend for Akonadi, GSoC '20)

Tue Jun 9 08:47:40 BST 2020

Hi Shashwat,

nice work so far! Good to see that you are progressing so well on the Akonadi 
side :)

Looking at the code, it looks good, except that you seem to be leaking memory 
from the raw pointers. for example in etesync_auth_get_token:

    const char *uname = charArrFromQString(username);
    const char *pass = charArrFromQString(password);
    char *token = etesync_auth_get_token(etesync, uname, pass);

Here charArrFromQString() allocates two new strings that you pass to the C 
function, but the function doesn't take ownership, so afterwards you just leak 
the memory. Unless the C functions take over ownership of the strings, you 
don't need to do any of the charrArrFromQString, you are perfectly fine doing

    char *token = etesync_auth_get_token(etesync, qPrintable(username), 
qPrintable(password));

qPrintable(s) is just a macro that calls s.toLatin1().constData() here (so 
basically convertintg QString to const char*, but the ownership of the data 
pointer to by the const char* remains in the QString (or rather the QByteArray 
temporary created by QString::toLatin1()). Anyway, this way the memory is only 
copied during the QString->QByteArray conversion, but remains managed by the 
QString/QByteArray, so there are no memory leaks.

Same applies for the char *token pointer that you get form the C function. You 
convert it to a QString, but that QString creates an internal copy of the 
string, so the data pointed to by "token" are leaked as well. Your 
QStringFromCharArr() should create the QString (btw use QString::fromLatin1() 
instead of QLatin1String()) and then free() the original pointer. Even better 
would be if you would immediately wrap the returned value into a 
std::unique_ptr so that you do not have to concern yourself with free()ing it, 
it will get free()'d automatically (you may want to google how to use 
std::unique_ptr with free() instead of the default delete)

Same then goes with the returned EteSyncCollectionInfo*, EteSyncSyncEntry* 
etc. pointers which you mostly forget to free: for this I would suggest the 
adaptor should never return a raw pointer unless the Etesync library remains 
an owner of it, and wrap all pointers that you are supposed to free yourself 
in std::unique_ptr so that you don't need to concern yourself with deleting 
them manually.

Nitpicks:
* class names should always start with a capital letter, e.g. EteSyncResource 
* coding style - use braces even for single-line if/for/... blocks 

If you have any questions, just ask :)

/Dan

On Friday, 5 June 2020 20:39:53 CEST Shashwat Jolly wrote:
> Hey everyone!
> 
> It's been a week since the coding period for GSoC started. Just wanted to
> share that I already have some code working for reading address books from
> EteSync.
> It still needs some work to implement the fetching asynchronously using
> jobs. Also, the configuration dialog hasn't been implemented and you would
> need to put in your EteSync credentials in the configure() function in
> etesyncresource.cpp to test it out.
> 
> I have my code up at
> https://invent.kde.org/sjolly/kdepim-runtime/-/tree/etesyncResource if you
> want to take a look.
> 
> More details here:
> https://thejollyblog.netlify.app/posts/KDE/gsoc-part-3-adding-etesync-addres
> sbooks
> 
> Feedback is always welcome!
> 
> Thanks!
> 
> --
> Shashwat Jolly
> <https://twitter.com/jollyshashwat>
> IRC: sjolly on Freenode
> https://twitter.com/jollyshashwat
> 
> On Tue, May 5, 2020 at 5:39 PM Shashwat Jolly <shashwat.jolly at gmail.com>
> 
> wrote:
> > Hi everyone!
> > 
> > I'm Shashwat Jolly, a mathematics graduate from IIT Guwahati, India. I
> > have been selected for the GSoC programme for the project *EteSync sync
> > backend for Akonadi*, and I'm pumped to get started! This email is for
> > introducing myself and my project to you all. :)
> > As I'm just starting with the project, my understanding may be pretty
> > basic, but here it goes:
> > 
> > *Akonadi* is the backend framework providing APIs for storage and
> > retrieval of the user's personal info such as contacts, email and
> > calendar.
> > These APIs are used by applications like Kontact, Kmail, KAddressBook and
> > many others. Akonadi also allows one to sync this data via a number of
> > services like Google, Microsoft Exchange, DAV servers and many others.
> > 
> > My project is to add to this list a secure, end-to-end encrypted, FLOSS
> > sync solution for contacts, calendars and tasks, called *EteSync*. EteSync
> > clients are available for Android, iOS, the desktop (Cal/CardDAV bridge)
> > and the web, and a Thunderbird plugin is in the works. The server too is
> > open-source and can be self-hosted. As mentioned, EteSync utilizes
> > end-to-end encryption, hence giving users the benefit of truly owning
> > their
> > data and respecting their privacy.
> > 
> > I'm well-versed with C++, and have made a few contributions to KDE PIM,
> > which also introduced me to Qt. I'm looking forward to working with all of
> > you!
> > 
> > Apart from technical stuff, I'm very interested in all the latest and
> > upcoming gadgets (mobiles, laptops etc). I'm into music, movies and twisty
> > puzzles (Rubik's cube and the like). If you're also into any of this
> > stuff,
> > or even if not, I'm *sjolly on IRC*. Hoping to interact and learn from
> > all of you!
> > 
> > Here's my project proposal. Please have a look:
> > https://drive.google.com/open?id=1nWU5yaG_Anpl6L_QvMal0Vf2g_sQtC56

-- 
Daniel Vrátil
www.dvratil.cz | dvratil at kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20200609/241f1da9/attachment.sig>