Skip to content

Fix MAC address parsing and serialization#55

Merged
rgwohlbold merged 4 commits intomainfrom
46-shift-exponent-32-is-too-large-for-32-bit-type-int
Mar 31, 2023
Merged

Fix MAC address parsing and serialization#55
rgwohlbold merged 4 commits intomainfrom
46-shift-exponent-32-is-too-large-for-32-bit-type-int

Conversation

@rgwohlbold
Copy link
Copy Markdown
Member

@rgwohlbold rgwohlbold commented Mar 31, 2023

Solves #46

  • Eliminate undefined behavior in MacAddress::number
  • Use network byte order (big endian) for MacAddress::number
  • Use correct length for substr in constructor
  • Write validation and serialization tests

I blacklisted the rule readability-function-cognitive-complexity globally since it flagged the test case without a reason, but I don't know whether that is the right call.

@rgwohlbold rgwohlbold requested a review from Sohn123 March 31, 2023 09:28
@Sohn123
Copy link
Copy Markdown
Member

Sohn123 commented Mar 31, 2023

Solves #46

* Eliminate undefined behavior in `MacAddress::number`

* Use network byte order (big endian) for `MacAddress::number`

* Use correct length for `substr` in constructor

* Write validation and serialization tests

I blacklisted the rule readability-function-cognitive-complexity globally since it flagged the test case without a reason, but I don't know whether that is the right call.

In the documentation of the check is stated that by default the complexity of macros also counts. Maybe disable macros and try again? If this doesn't help, I would disable this check with a block NOLINT directive instead of disabling it globally.

@rgwohlbold
Copy link
Copy Markdown
Member Author

Thanks for the hint! I updated .clang-tidy to ignore macros for this rule.

Copy link
Copy Markdown
Member

@Sohn123 Sohn123 left a comment

Choose a reason for hiding this comment

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

Nice that you added some tests to the parsing :) I think disabling the macros for this check is a fair call since macros are not a problem for human complexity in my opinion. So I would say let's fix this bug and :shipit:

@rgwohlbold rgwohlbold merged commit 282ba85 into main Mar 31, 2023
@rgwohlbold rgwohlbold deleted the 46-shift-exponent-32-is-too-large-for-32-bit-type-int branch March 31, 2023 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants