Skip to content

core: move NTOHL and friends into byteorder.h#1984

Merged
OlegHahm merged 3 commits intoRIOT-OS:masterfrom
Kijewski:net_help-is-odd
Nov 19, 2014
Merged

core: move NTOHL and friends into byteorder.h#1984
OlegHahm merged 3 commits intoRIOT-OS:masterfrom
Kijewski:net_help-is-odd

Conversation

@Kijewski
Copy link
Copy Markdown
Contributor

@Kijewski Kijewski commented Nov 10, 2014

No description provided.

@Kijewski Kijewski added Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: network Area: Networking Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer labels Nov 10, 2014
@Kijewski Kijewski added this to the Release NEXT MAJOR milestone Nov 10, 2014
@Kijewski Kijewski mentioned this pull request Nov 10, 2014
Closed
OlegHahm
OlegHahm reviewed Nov 10, 2014
View reviewed changes
core/include/byteorder.h
Copy link
Copy Markdown
Member

@OlegHahm OlegHahm Nov 10, 2014

Choose a reason for hiding this comment

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

Weird indentation.

Copy link
Copy Markdown
Contributor Author

@Kijewski Kijewski Nov 10, 2014

Choose a reason for hiding this comment

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

Same indentation as the other comments.

Copy link
Copy Markdown
Member

@OlegHahm OlegHahm Nov 10, 2014

Choose a reason for hiding this comment

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

Then they are all weird. ;-b

Copy link
Copy Markdown
Contributor Author

@Kijewski Kijewski Nov 10, 2014

Choose a reason for hiding this comment

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

You call it weird, I call it readable. :)

Copy link
Copy Markdown
Contributor Author

@Kijewski Kijewski Nov 10, 2014

Choose a reason for hiding this comment

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

The proper answer is probably that I indented everything to the length of @param[in,out] so even if an in-out parameter was added, everything would still be properly aligned.

Copy link
Copy Markdown
Member

@miri64 miri64 Nov 10, 2014

Choose a reason for hiding this comment

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

Setting of the param block with some newlines would improve the readability, I guess.

Copy link
Copy Markdown
Contributor Author

@Kijewski Kijewski Nov 11, 2014

Choose a reason for hiding this comment

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

Maybe, but that's not part of this PR.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Nov 11, 2014

Big brother is watching you btw ;-)

@miri64
Copy link
Copy Markdown
Member

miri64 commented Nov 11, 2014

ACK

@Kijewski
Copy link
Copy Markdown
Contributor Author

Kijewski commented Nov 11, 2014

@LudwigOrtmann, wanna review?

@miri64 miri64 mentioned this pull request Nov 11, 2014
Closed
@Kijewski Kijewski assigned OlegHahm and unassigned miri64 Nov 12, 2014
@miri64
Copy link
Copy Markdown
Member

miri64 commented Nov 14, 2014

Can we get a second review? It's a core change.

miri64 added a commit to miri64/RIOT that referenced this pull request Nov 14, 2014
@miri64
Merge branch 'net_help-is-odd' (RIOT-OS#1984) into 6lowpan-fix-major
e747948
@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Nov 14, 2014

Counter question: Has anyone tested with a real network application, preferable on real hardware?

@Kijewski
Copy link
Copy Markdown
Contributor Author

Kijewski commented Nov 14, 2014

You could, I guess. :)

@miri64
Copy link
Copy Markdown
Member

miri64 commented Nov 14, 2014

I tested byteorder_hton*/byteorder_ntoh* already back when we merged #1699 the module with wireshark and native, and yes I already use it in the new network stack where it also works like a charm. This PR however just moves the HTON*/NTOH* macros (the type-unsave wrappers for byteorder_hton*/byteorder_ntoh*), that were already in use in the old network stack, from the net_help.h header to the byteorder.h (+ removal of some unnecessary redefinitions).

@miri64 miri64 mentioned this pull request Nov 17, 2014
Merged
@Kijewski
Copy link
Copy Markdown
Contributor Author

Kijewski commented Nov 18, 2014

rebased

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Nov 19, 2014

Tested and worked: ACK

OlegHahm added a commit that referenced this pull request Nov 19, 2014
@OlegHahm
Merge pull request #1984 from Kijewski/net_help-is-odd
771b3a4 
core: move NTOHL and friends into byteorder.h
@OlegHahm OlegHahm merged commit 771b3a4 into RIOT-OS:master Nov 19, 2014
@Kijewski Kijewski deleted the net_help-is-odd branch November 19, 2014 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@OlegHahm OlegHahm

Labels

Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: network Area: Networking Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer

Projects

None yet

Milestone

Release 2014.12

Development

Successfully merging this pull request may close these issues.

3 participants

@Kijewski @miri64 @OlegHahm