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

pa_converters include math if PA_USE_C99_LRINTF #389

Closed
wants to merge 1 commit into from

Conversation

jmelas
Copy link
Contributor

@jmelas jmelas commented Dec 22, 2020

pa_converters.c did not include math.h in case PA_USE_C99_LRINTF is defined
so the compiler could not find the definition for lrintf

Copy link
Collaborator

@RossBencina RossBencina left a comment

Choose a reason for hiding this comment

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

Seems fine. Although the actual use of lrint looks suspect to me.

@RossBencina
Copy link
Collaborator

FYI here's what I think about the lrintf usage: #390 I wouldn't enable it.

@RossBencina RossBencina added the src-common Common sources in /src/common label Dec 22, 2020
@RossBencina
Copy link
Collaborator

This is pending some consensus on #390. I think either we merge this PR or we strip out the PA_USE_C99_LRINTF code.

@philburk
Copy link
Collaborator

I think we should close this and then remove the PA_USE_C99_LRINTF code.

@RossBencina
Copy link
Collaborator

I've submitted #403 as an alternative to this. I'll close this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
src-common Common sources in /src/common
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants