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

Add support for OS X #24

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add support for OS X #24

wants to merge 3 commits into from

Conversation

ShaharHD
Copy link

@ShaharHD ShaharHD commented May 6, 2020

  • obstack added as an optional build library (regardless of OS)
    To activate use ./configure --enable-obstack
  • application if on OS X will use the correct getprogname
  • removed obstack and now using malloc/free

- obstack added as an optional build library (regardless of OS)
- application if on OS X will use the correct getprogname
@ShaharHD ShaharHD mentioned this pull request May 6, 2020
@lechium
Copy link

lechium commented May 7, 2020

builds for me now! thanks!! great work :D

@spbnick
Copy link
Member

spbnick commented May 9, 2020

Thank you, @ShaharHD, very nice work. However, I'd rather not add obstack source code to hidrd. I'd keep this PR open for others to pick up, but would prefer the code switched to malloc and remove the dependency altogether.

@ShaharHD
Copy link
Author

ShaharHD commented May 9, 2020

@spbnick I'll change the PR to use malloc rather then incorporating obstack.

Convert from using obstack to use malloc and free
No need to check for NULL on next before calling
@ShaharHD
Copy link
Author

@spbnick I've removed obstack as requested.

@spbnick
Copy link
Member

spbnick commented May 20, 2020

Thank you, @ShaharHD, I looked through it quickly, and it looks good, but need to find time for an in-depth review still.

@spbnick spbnick self-assigned this May 20, 2020
@ShaharHD
Copy link
Author

@spbnick any updates?

@spbnick
Copy link
Member

spbnick commented Jun 28, 2020

Hi @ShaharHD, I took a deeper look at the PR, and would like to ask you to get rid of program_invocation(_short)_name properly, instead of just defining it to be a pointer to a function cast to string. A good approach could be implementing the GNU version of basename and using that on argv[0] instead.

Please also rebase on #26 to pick up OS X test fixes, and to have CI for your changes.

@ShaharHD
Copy link
Author

@spbnick just to confirm, as it seems you already rebased my code, take https://github.com/spbnick/hidrd/tree/add_osx_support as my base?

@ShaharHD
Copy link
Author

Hi @ShaharHD, I took a deeper look at the PR, and would like to ask you to get rid of program_invocation(_short)_name properly, instead of just defining it to be a pointer to a function cast to string. A good approach could be implementing the GNU version of basename and using that on argv[0] instead.

As OS X already has the implemantion, and it seems just like a GNU vs OS X mapping, another option can be like

#undef GET_PROGRAM_NAME
#ifdef __GLIBC__
#   define GET_PROGRAM_NAME() program_invocation_short_name
#else /* *BSD and OS X */
#   include <stdlib.h>
#   define GET_PROGRAM_NAME() getprogname()
#endif

And use GET_PROGRAM_NAME() - as it seems just a different name for the exact same funcion.

@spbnick
Copy link
Member

spbnick commented Jun 29, 2020

@spbnick just to confirm, as it seems you already rebased my code, take https://github.com/spbnick/hidrd/tree/add_osx_support as my base?

Yep!

@spbnick
Copy link
Member

spbnick commented Jun 29, 2020

Perhaps this could be made more portable and cleaner by using getprogname from libbsd on Linux and directly from the system on OS X.

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.

3 participants