Skip to content

Conversation

@marek22k
Copy link
Contributor

@marek22k marek22k commented Dec 4, 2025

On modern systems, time values may be stored in a 64-bit integer. To send this over the network, it is necessary to package it appropriately. htonl is insufficient for this purpose, as it is limited to 32-bit values.

See also https://stackoverflow.com/questions/3022552/is-there-any-standard-htonl-like-function-for-64-bits-integers-in-c

@marek22k
Copy link
Contributor Author

marek22k commented Dec 4, 2025

@troglobit As discussed via email, here is the PR for the bug.

Copy link
Owner

@troglobit troglobit left a comment

Choose a reason for hiding this comment

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

I would perhaps have broken out to two helper functions marshal()/unmarshal(), but this works too.

I'm approving this, but if you want to address my inline comment before we merge this, it's up to you.

mping.c Outdated
Comment on lines 834 to 840
if (debug)
{
struct mping packet;
printf("tv_sec size: %zu\n", sizeof(packet.tv.tv_sec));
printf("tv_usec size: %zu\n", sizeof(packet.tv.tv_usec));
}

Copy link
Owner

Choose a reason for hiding this comment

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

Looks like developer debug message, maybe we can do without it? Also, notice coding style (brace placement)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually overlooked the brackets because they were different in the functions.
I have now corrected this and shortened the debug output to one line.

On modern systems, time values may be stored in a 64-bit integer. To send this over the network, it is necessary to package it appropriately. htonl is insufficient for this purpose, as it is limited to 32-bit values.

See also https://stackoverflow.com/questions/3022552/is-there-any-standard-htonl-like-function-for-64-bits-integers-in-c

Signed-off-by: Marek Küthe <m.k@mk16.de>
@marek22k
Copy link
Contributor Author

marek22k commented Dec 4, 2025

I would perhaps have broken out to two helper functions marshal()/unmarshal(), but this works too.

You can still do that. I just adopted the existing structure.

Signed-off-by: Marek Küthe <m.k@mk16.de>
@marek22k
Copy link
Contributor Author

marek22k commented Dec 4, 2025

I have now extracted the network encoding code into two methods. If you would like it differently, you can just merge the first commit.
I hope the style is correct this time - if not, please let me know. And I hope I haven't removed too much seq_no anywhere. I tested the program locally with IPv6. It works there.

Copy link
Owner

@troglobit troglobit left a comment

Choose a reason for hiding this comment

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

Nice work, thanks!

@troglobit troglobit merged commit 19df027 into troglobit:main Dec 4, 2025
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