-
Notifications
You must be signed in to change notification settings - Fork 721
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.
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,28 @@ | |
/// @brief The main namespace for the PcapPlusPlus lib | ||
namespace pcpp | ||
{ | ||
/// @defgroup ProtocolTypes ProtocolType and ProtocolTypeFamily | ||
/// 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I'm not sure this section should be defined as There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 This is why I put that section as internal documentation. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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... There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
/// | ||
/// 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; | ||
|
@@ -223,6 +245,8 @@ namespace pcpp | |
/// Diagnostic over IP protocol (DOIP) | ||
const ProtocolType DOIP = 59; | ||
|
||
/// @} | ||
|
||
/// An enum representing OSI model layers | ||
enum OsiModelLayer | ||
{ | ||
|
@@ -250,6 +274,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); | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.