maildev@lists.thunderbird.net

Thunderbird email developers

View all threads

Convert libmime from C to C++

BB
Ben Bucksch
Wed, May 23, 2018 8:24 PM

Patrick Brunschwig wrote on 23.05.18 08:39:

The entry barrier for new developers is_very_  high, particularly
because of the the uncommon C-based object structure, and maintenance is
not really easy either. Only a very small number of developers
understand enough of libmime to be able to work on it.

By providing an API that is easier to understand, we could make it
easier for new developers to understand how libmime works, and we could
benefit from easier maintenance in the future.

Jörg wrote in followup:

No active developer understands libmime to be able to work on it.

Yes, that's exactly why I'm doing this. Even though I don't understand
all of libmime, I feel like I have enough of a conceptual handle on it
to be able to transform this into at least C++, even if we don't make
the jump to JS yet.

Please note my initial post. I specifically highlighted that this is a
limited scope project, and at least 10 times simpler than rewriting
libmime or replacing it.

Because I don't change the logic at all, the risk of regressions is
reduced mostly to typos. I hope the review will focus on /only/ that,
too. If there's anything completely systematically wrong in the concept,
a simple run should uncover it. My practical problem is that I need to
finish the transition completely before I can compile and run anything.

So anything that will make libmime a little less brittle with its
notorious function pointers and free'ing strategies (one crash was a
double free) should help us before we replace the whole thing.

That said, I hope that replacing the doubtful free'ing strategies
won't introduce a whole new lot of crashes.

Somebody suggested to also convert the *pointer member variables (like
DisplayOptions etc.) into normal straight member variables, allocated as
part of the holding class. I think that's a great idea and will get rid
of a lot of the MALLOC and FREE and pointer dereferencing and null checks.

That would be a second step, though, either before or after or during
the code style change. It's too much change for me at the moment. It's
already too much to chew for me, so I'm keeping everything not strictly
necessary for compiling and working out of this first phase.

I may even need some help with the straight conversion as it is. We'll
see how it goes.

Thanks for the encouraging thoughts. This is important for me that I
have buy-in.

Ben

Patrick Brunschwig wrote on 23.05.18 08:39: > The entry barrier for new developers is_very_ high, particularly > because of the the uncommon C-based object structure, and maintenance is > not really easy either. Only a very small number of developers > understand enough of libmime to be able to work on it. > > By providing an API that is easier to understand, we could make it > easier for new developers to understand how libmime works, and we could > benefit from easier maintenance in the future. Jörg wrote in followup: > No active developer understands libmime to be able to work on it. Yes, that's exactly why I'm doing this. Even though I don't understand all of libmime, I feel like I have enough of a conceptual handle on it to be able to transform this into at least C++, even if we don't make the jump to JS yet. Please note my initial post. I specifically highlighted that this is a limited scope project, and at least 10 times simpler than rewriting libmime or replacing it. Because I don't change the logic at all, the risk of regressions is reduced mostly to typos. I hope the review will focus on /only/ that, too. If there's anything completely systematically wrong in the concept, a simple run should uncover it. My practical problem is that I need to finish the transition completely before I can compile and run anything. > So anything that will make libmime a little less brittle with its > notorious function pointers and free'ing strategies (one crash was a > double free) should help us before we replace the whole thing. > > That said, I hope that replacing the doubtful free'ing strategies > won't introduce a whole new lot of crashes. Somebody suggested to also convert the *pointer member variables (like DisplayOptions etc.) into normal straight member variables, allocated as part of the holding class. I think that's a great idea and will get rid of a lot of the MALLOC and FREE and pointer dereferencing and null checks. That would be a second step, though, either before or after or during the code style change. It's too much change for me at the moment. It's already too much to chew for me, so I'm keeping everything not strictly necessary for compiling and working out of this first phase. I may even need some help with the straight conversion as it is. We'll see how it goes. Thanks for the encouraging thoughts. This is important for me that I have buy-in. Ben
BB
Ben Bucksch
Wed, May 23, 2018 8:27 PM

Ben Bucksch wrote on 23.05.18 22:22:

Tom Prince wrote on 23.05.18 08:58:

Even if that is the case, it seems like it would make sense to
implement an API that answers "what is the body" on top of one that
generates a mime tree.

We could of course build an even higher level API that just reads a
messages, returns only the body (in either HTML or sanitized HTML or
plaintext), and throws everything else away. If there are many callers
that need only that, it would make sense.

If we ever make MailExtensions (akin WebExtensions), this would be a
candidate function.

Ben

Ben Bucksch wrote on 23.05.18 22:22: > Tom Prince wrote on 23.05.18 08:58: >> Even if that is the case, it seems like it would make sense to >> implement an API that answers "what is the body" on top of one that >> generates a mime tree. > > > bool IsMessageBody(); > > https://phabricator.services.mozilla.com/D1344#change-toxB7gkcC6hQ > > ;-) > > We could of course build an even higher level API that just reads a messages, returns only the body (in either HTML or sanitized HTML or plaintext), and throws everything else away. If there are many callers that need only that, it would make sense. If we ever make MailExtensions (akin WebExtensions), this would be a candidate function. Ben