DIGImend / hidrd

HID report descriptor I/O library and conversion tool
GNU General Public License v2.0
167 stars 29 forks source link

Add support for OS X #24

Open ShaharHD opened 4 years ago

ShaharHD commented 4 years ago
lechium commented 4 years ago

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

spbnick commented 4 years ago

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 commented 4 years ago

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

ShaharHD commented 4 years ago

@spbnick I've removed obstack as requested.

spbnick commented 4 years ago

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

ShaharHD commented 4 years ago

@spbnick any updates?

spbnick commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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