Skip to content

Added documentation comment for protocol type and protocol type family. #1919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 14, 2025
Merged

Added documentation comment for protocol type and protocol type family. #1919

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b439377
Added documentation comment for protocol type and protocol type family.
Dimi1010 Aug 13, 2025
aba3a39
Lint
Dimi1010 Aug 13, 2025
0b98813
Merge remote-tracking branch 'upstream/dev' into docs/protocol-type-docs
Dimi1010 Aug 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions Packet++/header/ProtocolType.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,28 @@
/// @brief The main namespace for the PcapPlusPlus lib
namespace pcpp
{
/// @defgroup ProtocolTypes ProtocolType and ProtocolTypeFamily
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better if we could do the check using the compiler, but if not, we reviewers need to read the code carefully.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be, but we need cpp17 for that.

/// In the PcapPlusPlus library, specific protocols are identified by a `ProtocolType` value, which is an 8-bit
/// unsigned integer. Each Layer class has a ProtocolType that can be used to identify the protocol of that layer.
///
/// As there are some situations where multiple protocols can be grouped together (e.g. IPv4 and IPv6 into IP),
/// PcapPlusPlus also defines ProtocolTypeFamily, which is a group of multiple protocol types. In functions where a
/// ProtocolTypeFamily parameter is expected, a single ProtocolType value can be passed instead. The library will
/// treat it as a family containing only that single protocol.
///
/// @internal
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'm not sure this section should be defined as @internal

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I marked it as such, as I think it would be beneficial for those 2 typedefs to be converted to actual structs for type safety eventually.

The current issue with that is their usages in switch statemets. To use a structure in a case XXX: stmt requires the structure to be castable to integral type at compile time. To do this would require all protocol definitions to be constexpr variables, which has some issues when in headers pre-cpp17.

This is why I put that section as internal documentation.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion, just thought this information could be useful for everyone using this library, not just its developers. If you feel strongly about keeping it, I'm ok with it too 🙂

The PR is ready to merge, so whatever you think...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm gonna keep it internal for now. We can publish more documentation on it if we move to discrete types instead of aliases. As is, the users shouldn't really depend on any internal relations on how it works, as the actual public use cases for ProtocolType and ProtocolTypeFamily are as a more complicated Enum tbh.

///
/// A ProtocolTypeFamily pack is represented as a 32-bit unsigned integer. Each octet in the 32-bit value is a
/// ProtocolType, allowing for up to 4 protocols to be packed into a single ProtocolTypeFamily value. For example,
/// the ProtocolTypeFamily for IP is represented as 0x203, which contains both IPv4 (0x02) and IPv6 (0x03)
/// protocols. A single ProtocolType casted to ProtocolTypeFamily will result the value '0x000000xx' where 'xx' is
/// the value of the ProtocolType.
///
/// @endinternal

/// @addtogroup ProtocolTypes
/// @{

/// @typedef ProtocolType
/// Representing all protocols supported by PcapPlusPlus
typedef uint8_t ProtocolType;
Expand Down Expand Up @@ -229,6 +251,8 @@ namespace pcpp
/// FTP protocol family (FTPControl and FtpData protocols)
const ProtocolTypeFamily FTP = 0x3c29;

/// @}

/// An enum representing OSI model layers
enum OsiModelLayer
{
Expand Down Expand Up @@ -256,6 +280,7 @@ namespace pcpp
/// @param family A protocol type family value.
/// @param protocol A protocol type value to check against the family.
/// @return True if the protocol is part of the family, false otherwise.
/// @ingroup ProtocolTypes
constexpr bool protoFamilyContainsProtocol(ProtocolTypeFamily family, ProtocolType protocol)
{
auto const protocolToFamily = static_cast<ProtocolTypeFamily>(protocol);
Expand Down
Loading