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

cmake, sa: enable unix sockets, if HAVE_UNIXSOCK is undefined #636

Merged
merged 1 commit into from
Jan 9, 2023

Conversation

fAuernigg
Copy link
Contributor

@fAuernigg fAuernigg commented Jan 9, 2023

Unix sockets are turned on per default in cmake and are used in header re_sa.h. This PR enables the unix socket flag, if not explicitly defined during builds of external libs using re_sa.h.

Edit by @cspiel1 : Relates to baresip/baresip#2402

@fAuernigg
Copy link
Contributor Author

I agree that positive logic would be better but is not a solution here.

If sa struct is used via libre api's but from external libaries, a missing "HAVE_UNIXSOCK" produces a very hard find segv: 'stack smashed" in my case.
If a missing HAVE_UNIXSOCK would lead to a compile time error this would be ok for me. A not debug'able crash at runtime due to a missing definement or updated lib re default value for HAVE_UNIXSOCK is really ugly.

I had to add printfs arround the code smashing the stack to finally find out, which definement and which default value was missing in my library.

@sreimers
Copy link
Member

sreimers commented Jan 9, 2023

Looks good to me (ignore my deleted comment)

@fAuernigg
Copy link
Contributor Author

This PR handles 3 states of USE_UNIXSOCK / HAVE_UNIXSOCK.
ON, OFF, undefined (in third party libs using re_sa.h but having no clue of the default value)

@sreimers sreimers added the bug Something isn't working label Jan 9, 2023
@sreimers sreimers added this to the v2.11.0 milestone Jan 9, 2023
src/sa/sa.c Outdated Show resolved Hide resolved
src/sa/sa.c Outdated Show resolved Hide resolved
@fAuernigg fAuernigg force-pushed the cmake_sa_have_unix_socket branch 2 times, most recently from 260db6e to 0bf7064 Compare January 9, 2023 10:21
@fAuernigg fAuernigg changed the title cmake, sa: enable unix sockets, if HAVE_UNIXSOCK turned on or is unde… cmake, sa: enable unix sockets, if HAVE_UNIXSOCK is undefined Jan 9, 2023
Unix sockets are turned on per default in cmake and are used in header re_sa.h
Enable unix socket flag, if not defined during build of external code
including re_sa.h.
@fAuernigg fAuernigg marked this pull request as ready for review January 9, 2023 10:33
@sreimers sreimers merged commit e3e3b37 into baresip:main Jan 9, 2023
@fAuernigg fAuernigg deleted the cmake_sa_have_unix_socket branch January 9, 2023 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants