Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

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

Merged
merged 3 commits into from
Nov 19, 2014
Merged

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

merged 3 commits into from
Nov 19, 2014

Conversation

Kijewski
Copy link
Contributor

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
/**
* @brief Convert from host byte order to network byte order, 16 bit.
* @see byteorder_htons()
* @param[in] v The integer to convert.
Copy link
Member

Choose a reason for hiding this comment

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

Weird indentation.

Copy link
Contributor Author

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
Member

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
Contributor Author

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
Contributor Author

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
Member

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
Contributor Author

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
Member

miri64 commented Nov 11, 2014

Big brother is watching you btw ;-)

@miri64
Copy link
Member

miri64 commented Nov 11, 2014

ACK

@Kijewski
Copy link
Contributor Author

@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
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
Member

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

@Kijewski
Copy link
Contributor Author

You could, I guess. :)

@miri64
Copy link
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
Contributor Author

rebased

@OlegHahm
Copy link
Member

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
# for free to join this conversation on GitHub. Already have an account? # 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