Skip to content
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

osx: replace getnameinfo with inet_ntop #464

Conversation

correabuscar
Copy link
Contributor

@correabuscar correabuscar commented Nov 6, 2022

this is like PR #462 for FreeBSD,
and like PR #458 for Linux.


I can't test the build and run for these, as I've been unable to get macOS in virtualbox (tried 4 iso`s). But I'll look in github actions for the build, and just hope the runs work fine on bare metal.

outdated info

Unsure about those #include`s: is #include <arpa/inet.h> the same as #include <sys/arpa/inet.h> ?
I've got them from:

LEGACY SYNOPSIS
     #include <sys/types.h>
     #include <sys/socket.h>
     #include <sys/netinet/in.h>
     #include <sys/arpa/inet.h>

     These include files are necessary for all functions.

src: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/inet_ntop.3.html

PS: Thank you for this learning experience btw :) (with all the PRs, not just this one)

@correabuscar correabuscar force-pushed the osx_replace__getnameinfo__with__inet_ntop branch from 7822eb4 to 41d5818 Compare November 6, 2022 13:45
src/osx/btop_collect.cpp Outdated Show resolved Hide resolved
this is like PR aristocratos#462 for FreeBSD,
and like PR aristocratos#458 for Linux.
@correabuscar correabuscar force-pushed the osx_replace__getnameinfo__with__inet_ntop branch from 41d5818 to f4eea3f Compare November 6, 2022 13:56
Copy link
Owner

@aristocratos aristocratos left a comment

Choose a reason for hiding this comment

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

Tested to work as expected on macos13, since it compiles fine on macos11 and macos12 I would assume the same is true there.

Great work 👍🏼
Thanks for your contributions!

@aristocratos aristocratos merged commit 9dc5753 into aristocratos:main Nov 6, 2022
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