Skip to content

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