Port the code to C++11

[seladb]

We recently moved to C++11 and big parts of the code are still using C++98 style.
This will be an ongoing task to port more and more parts of the code to C++11.

For example:

• Use nullptr instead of NULL
• Use auto where possible
• Use enum class more
• Use range-based loops where possible
• Use delete, default, override where needed
• Use unordered_map instead of map for better performance
• etc.

IMPORTANT: this change can (and should) be done incrementally. Please try to open small PRs with not a lot of changes that are easy to review

[InfiniteVerma]

I'd like to help with this.

[egecetin]

Not directly related with C++11 but also, library have some many C style casts, which can be easily located by using cppcheck. Anyone who wants fix them just remove or comment cstyleCast:* line from cppcheckSuppressions.txt and run pre-commit run --all-files or manually run cppcheck from command line.

[jpcofr]

Hi @seladb - I would like to collaborate on this task. Is there any task breakdown already or is there any progress?
Also, why porting to C++11 instead of later versions? (is it because the latest port you mention in your first comment was to C++11?

UPDATE - based on the Release notes (changes from v22.05) it seems this issue/ticket should be closed...

[seladb]

Hi @seladb - I would like to collaborate on this task. Is there any task breakdown already or is there any progress? Also, why porting to C++11 instead of later versions? (is it because the latest port you mention in your first comment was to C++11?

UPDATE - based on the Release notes (changes from v22.05) it seems this issue/ticket should be closed...

Hi @jpcofr , thanks for offering your help! Actually, the reason this issue is still open is because this is an ongoing effort. We migrated to C++11 about a year ago, but a lot of the code base is still in C++98 style (definitely most of the code that was written more than a year ago). For example: we don't always use auto where needed, we still use std::map where we could use std::unordered_map, we don't use range-based loops in many cases, etc. We could use your help with those.

As I mentioned above - we don't expect a huge PR that fixes all of these things. We actually prefer small PRs that make incremental fixes, and we hope that we can slowly migrate as much of the code as possible.

It'd be really great if you can help. I'd like to thank you again and I'm looking forward to seeing your contributions! 🙏 🙏

[jpcofr]

be really great if you can help.

@seladb ok I can collaborate. I am new to this repo so I need to check out how is everything working. What would be the next namespace/class etc. that you would like to be ported? I can start from there...

[seladb]

be really great if you can help.

@seladb ok I can collaborate. I am new to this repo so I need to check out how is everything working. What would be the next namespace/class etc. that you would like to be ported? I can start from there...

@jpcofr If you want you can start with HttpAnalyzer. It's a quite simple utility (or an example of how to use PcapPlusPlus), and it uses a std::map that can be converted to std::unordered_map. It also has quite a few for loops that can be modified, and I'm sure we can use the auto keyword more.
This utility has python-based tests that you can run locally, and they also run in GitHub Actions as part of the CI: https://github.com/seladb/PcapPlusPlus/actions/runs/5988356929/job/16243457492#step:8:65

Please let me know if you need any help

[tigercosmos]

btw, maybe we can change the issue to C++20 directly.

[seladb]

btw, maybe we can change the issue to C++20 directly.

@tigercosmos for now I prefer to keep supporting C++11 to support a broader variety of platforms and devices

[Dimi1010]

Another thing is replacing ostringstream usages with to_string where it is used to convert only a single value to string.

[rtmeng]

Hi @seladb , I would also like to collaborate on this, is it still open?

[seladb]

Hi @seladb , I would also like to collaborate on this, is it still open?

Yes sure, you're more than welcome to contribute! 🙏
Feel free to open PRs and we'll review

[rtmeng]

Hi @seladb , I would also like to collaborate on this, is it still open?

Yes sure, you're more than welcome to contribute! 🙏 Feel free to open PRs and we'll review

Great! I'm still looking through the repo but is there anywhere I should start for what should be ported next?

[tigercosmos]

Hi @seladb , I would also like to collaborate on this, is it still open?

Yes sure, you're more than welcome to contribute! 🙏 Feel free to open PRs and we'll review

Great! I'm still looking through the repo but is there anywhere I should start for what should be ported next?

No, you can start at any place that uses C++98 instead of C++11. You can also refer to some merged PR for references.

[seladb]

Hi @rtmeng ! I just replied to your email. If you're interested in something nice and simple to get into the project, you can try modernizing the tutorials code

[tigercosmos]

@Dimi1010 What are your thoughts on how much we still have to finish?

[Dimi1010]

@Dimi1010 What are your thoughts on how much we still have to finish?

@tigercosmos

I think the map -> unordered_map is almost done. The only place I can find a #include <map> is in TcpReassembly.h and TextBasedProtocol.h, tho I haven't checked if that is deliberate.

For C-casts, I clean them up as I find them, but there are still plenty to go around... and it sucks you can't find them easily with a find command...

Two things I might add to the list:

• Increased preference on using unique_ptr and shared_ptr over the raw pointers for dynamic memory management.
• Increased preference of "pass-by-value + std::move" instead of const T& for constructor parameters that are just copied into member fields, as that would allow moving the parameter into the member field via double move than copying even when its not strictly required. Reasoning: https://stackoverflow.com/a/51706522/11922936
• • Setup of perfect forwarding is also an option, but its more of a hassle and requires templates.

[egecetin]

@Dimi1010 For c style casts you can use cppcheck. It should point most of them. Just remove cstyleCast:* line from cppcheckSuppressions.txt and run pre-commit locally or directly cppcheck. It is up to you which one you run

[lumiZGorlic]

Hi, i am trying to familiarise myself with the code base so i can later contribute with some tasks. This looks like a good way to start so i'd be happy to port some code to C++11 if that's OK ? I'll look at files in Packet++ first. if there's anything specific that should be done first please let me know.

[seladb]

Hi, i am trying to familiarise myself with the code base so i can later contribute with some tasks. This looks like a good way to start so i'd be happy to port some code to C++11 if that's OK ? I'll look at files in Packet++ first. if there's anything specific that should be done first please let me know.

Hi @lumiZGorlic ! Thank you so much for considering contributing to this project! 🙏
Packet++ is a good place to start. You can choose whatever protocol (layer) you want, I'd start with the simpler ones to familiarize yourself with the code-base. Please feel free to reach out to use if you have any question

[tigercosmos]

@lumiZGorlic The code under Examples also has room for improvement, and it should be easier for beginners. For example, you can start with refactoring the C-style casting to C++ casting first.

[DeepakReddy1999]

Hi @seladb ,I would like to contribute for the Examples and get familiar with the code base

[seladb]

Hi @seladb ,I would like to contribute for the Examples and get familiar with the code base

Sure @DeepakReddy1999 you're more than welcome to contribute! 🙏

[DeepakReddy1999]

Hi @seladb .There are still some NULLs in the code. Should we change it?

[tigercosmos]

@DeepakReddy1999 Yes, please go ahead.

[seladb]

Hi @seladb .There are still some NULLs in the code. Should we change it?

There are very few instances of NULL left, mostly in DpdkTests and KniTests. Please notice that we decided to keep NULL when calling Windows API like here: https://github.com/seladb/PcapPlusPlus/blob/master/Examples/IcmpFileTransfer/IcmpFileTransfer-pitcher.cpp#L40-L45

[egecetin]

For Windows API related NULLs, maybe a comment should be added to all of them to prevent future changes? With reference to MSDN page @seladb shared recently.

[tigercosmos]

I agree with @egecetin.